GNOME Bugzilla – Bug 646663
use g_get_user_runtime_dir
Last modified: 2012-04-25 19:04:11 UTC
It would be nice to use the XDG basedir user runtime dir instead of home for .gconfd.
Created attachment 185073 [details] [review] Remove unused daemon and lock directory functions They were made irrelevant by commit: 1196f4aa6da632ceafd7bb2c9d19c0a2f9c403ae
Created attachment 185074 [details] [review] Remove gconf_win32_get_home_dir() g_get_home_dir should give you the right thing.
Created attachment 185075 [details] [review] Use user runtime dir for saved state
So things have changed a bit since these patches were written. See: http://git.gnome.org/browse/gconf/commit/?id=ea6894303f7ebadb98bf4bf70a79c5bf6ef7cb90 and http://git.gnome.org/browse/gconf/commit/?id=2895a09bc0666cf7b26190bdb648ad1f52c174da (from bug 652289) We should still drop the unused gconf_get_lock_dir function. I'm on the fence about gconf_get_daemon_dir. The code currently makes it use the xdg runtime dir, but it would be fine to ditch it, and just have the caller use it like you propose.
Created attachment 212801 [details] [review] Use g_get_user_runtime_dir convenience function
Created attachment 212802 [details] [review] Remove unused gconf_get_lock_dir function
Created attachment 212804 [details] [review] Remove gconf_win32_get_home_dir() g_get_home_dir should give you the right thing.
Created attachment 212805 [details] [review] Make directories with parents in case they don't exist
Created attachment 212806 [details] [review] Use user runtime dir for daemon directory
Created attachment 212807 [details] [review] Use user runtime dir for lock sanity check
Review of attachment 212801 [details] [review]: I know I said on irc that it probably should use g_get_user_runtime_dir() but thinking about it more I kind of disagree now. ::: gconf/gconf-internals.c @@ -2810,1 @@ g_get_user_runtime_dir () returns g_get_user_cache_dir () not g_get_tmp_dir () in the case XDG_RUNTIME_DIR isn't set. That means in some cases this code effectively changes to: if (should put gconfd stuff in directory local to machine) { dir = directory on remote nfs server } Also, it changes the default location on windows, which probably isn't a big deal, but not really worth the churn either imo.
Review of attachment 212802 [details] [review]: cool
Review of attachment 212804 [details] [review]: ::: gconf/gconf-internals.c @@ -710,3 @@ - for (p = home_copy; *p; p++) - if (*p == '\\') - *p = '/'; I do see g_strdelimit(g_home_dir, "\\", '/'); inside gutils.c but that apparently is only in an OS/2 codepath. Do we know for sure we're going to get forward slashes or that other parts of the gconf code are okay with backslashes ? I wonder why the function exists if it's not needed.
Review of attachment 212805 [details] [review]: sure
Review of attachment 212806 [details] [review]: ::: gconf/gconf-internals.c @@ +2783,3 @@ else { + return g_strconcat (g_get_user_runtime_dir (), "/gconfd", NULL); if we're going to do this, maybe we shouldn't have GCONF_GLOBAL_LOCKS at all? Should probably use g_build_filename not g_strconcat (though the fact it previously used g_strconcat might have something to do with why gconf_win32_get_homedir does the slash conversion
Review of attachment 212807 [details] [review]: ::: gconf/gconf-sanity-check.c @@ +155,3 @@ else { + testfile = g_build_filename (g_get_user_runtime_dir (), again if we're going to use a local path in the global path half of a conditional we should consider just dropping the conditional. Actually we should consider trying gconf-sanity-check in it's entirety.
Created attachment 212825 [details] [review] Remove unused gconf_get_lock_dir function
Created attachment 212826 [details] [review] Make directories with parents in case they don't exist
Created attachment 212827 [details] [review] Use user runtime dir for daemon directory
Created attachment 212828 [details] [review] Use user runtime dir for lock sanity check
Comment on attachment 212801 [details] [review] Use g_get_user_runtime_dir convenience function Drop.
Comment on attachment 212804 [details] [review] Remove gconf_win32_get_home_dir() Drop
Review of attachment 212825 [details] [review]: looks fine
Review of attachment 212826 [details] [review]: fine
Comment on attachment 212827 [details] [review] Use user runtime dir for daemon directory Drop
Comment on attachment 212828 [details] [review] Use user runtime dir for lock sanity check Drop