GNOME Bugzilla – Bug 785260
gio/tests/appmonitor fails if local dir not writeable
Last modified: 2017-08-23 10:28:47 UTC
Another user on my system reported a test failure, so he chmod +755 so I could look. I cd'ed into his build dir and ran ./appmonitor. Even before the tests ran, I got diagnostics that indicated the tempdir used by the test was not created. No such problem for the original user or if I was in a directory that was writeable by me. appmonitor.c:main() uses g_mkdtemp() with a simple filename template, so it attempts to create the temp directory in the current directory where the test is run. If instead it used g_dir_make_tmp(), it would create the temp directory in a place that any user can write, so it wouldn't be affected by local dir permissions. Either way, this function also seems to leak the 'path' variable (should g_free() it).
Created attachment 357867 [details] [review] tests: Fix leak in appmonitor test Spotted by Daniel Macks. Signed-off-by: Philip Withnall <withnall@endlessm.com>
Created attachment 357868 [details] [review] tests: Add temporary working directory for appmonitor test Rather than creating a temporary directory in the current directory (typically the builddir), then never deleting it; create one in the system /tmp directory, and clean it up properly afterwards. Signed-off-by: Philip Withnall <withnall@endlessm.com>
Created attachment 357869 [details] [review] tests: Add temporary working directory for monitor test Similarly to the previous commit, move the temporary directory for the monitor test from $(cwd) to the system temporary directory. Signed-off-by: Philip Withnall <withnall@endlessm.com>
Review of attachment 357867 [details] [review]: +1
Review of attachment 357868 [details] [review]: :thumbs_up:
Review of attachment 357869 [details] [review]: :confetti_ball:
(In reply to Emmanuele Bassi (:ebassi) from comment #6) > Review of attachment 357869 [details] [review] [review]: > > :confetti_ball: You have no idea how long I’ve been waiting for someone to ack one of my patches with this much style.
I've just been thwarted by Bugzilla's lack of emoji shorthand support; hopefully GitLab will fix this glaring omission.
Thanks for the review! :rainbowfrog: :octopus: Attachment 357867 [details] pushed as 3ce00b2 - tests: Fix leak in appmonitor test Attachment 357868 [details] pushed as 0f5b523 - tests: Add temporary working directory for appmonitor test Attachment 357869 [details] pushed as a60359a - tests: Add temporary working directory for monitor test