After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 763344 - g_get_user_runtime_dir(): ensure directory exists
g_get_user_runtime_dir(): ensure directory exists
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
: 743836 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-03-08 20:42 UTC by Allison Karlitskaya (desrt)
Modified: 2017-10-24 09:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
g_get_user_runtime_dir(): ensure directory exists (3.41 KB, patch)
2016-03-08 20:42 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Allison Karlitskaya (desrt) 2016-03-08 20:42:47 UTC
If the XDG_RUNTIME_DIR environment variable is set, we are being told by
the OS that this directory exists and is appropriately configured
already.  In the fallback case of ~/.cache/, however, the directory may
not yet exist.

Rework the logic of this function a little so that we only check for the
environment variable once.  If it is not set, we will fall back to the
cache directory, and mkdir() it to make sure that it exists.

Meanwhile, remove a statement from the reference documentation that
promises a warning in this case (which has never been true) and replace
it with a statement that applications can rely on the directory
existing.

This change prevents each user of this API from having to check for the
directory for themselves; an example of that can be seen in bug 763274.
Comment 1 Allison Karlitskaya (desrt) 2016-03-08 20:42:50 UTC
Created attachment 323436 [details] [review]
g_get_user_runtime_dir(): ensure directory exists
Comment 2 Colin Walters 2016-03-09 14:35:57 UTC
Review of attachment 323436 [details] [review]:

Looks OK to me, just one random comment that isn't a problem.

::: glib/gutils.c
@@ +1342,3 @@
+        {
+          /* No need to strdup this one since it is valid forever. */
+          dir = g_get_user_cache_dir ();

This one grabs a mutex, and while holding that calls g_get_home_dir() which is also inside a GOnce, which calls g_get_user_database_entry() which is *also* inside a GOnce...

I can't think of a situation where it'd be a real world problem but I think at some point things would be simpler if we just had a static _gutils_init_global_state() that did all of this.
Comment 3 Allison Karlitskaya (desrt) 2016-03-09 15:42:47 UTC
(In reply to Colin Walters from comment #2)
> I can't think of a situation where it'd be a real world problem but I think
> at some point things would be simpler if we just had a static
> _gutils_init_global_state() that did all of this.

We used to have that, but I changed it since it meant that calling a function like g_get_user_cache_dir() also did things like parsing /etc/passwd to find out the user's realname.

I think that nesting onces like this is not really a problem... and not really a source of any amount of complexity.

Thanks for the review.
Comment 4 Allison Karlitskaya (desrt) 2016-03-09 15:43:45 UTC
Attachment 323436 [details] pushed as 7c6141a - g_get_user_runtime_dir(): ensure directory exists
Comment 5 Philip Withnall 2017-10-24 09:58:27 UTC
*** Bug 743836 has been marked as a duplicate of this bug. ***