[Merge] lp:~jamesodhunt/upstart/add-session-file into lp:upstart
Steve Langasek
steve.langasek at canonical.com
Tue Jan 29 08:29:22 UTC 2013
Review: Approve
get_subdir (const char *dir, const char *suffix, int create)
{
[...]
nih_assert (dir != NULL);
[...]
if (dir && dir[0] == '/') {
[...]
Looks like a redundant check for dir's nullness.
+char *
+xdg_get_runtime_dir (void)
+{
+ char *dir;
+
+ dir = getenv ("XDG_RUNTIME_DIR");
+
+ if (dir && dir[0] == '/') {
+ mkdir (dir, INIT_XDG_PATH_MODE);
+ dir = nih_strdup (NULL, dir);
+ return dir;
+ }
+
+ return dir;
+}
As mentioned, the session init won't have access to delete the directory... or to create it. Given that you're not checking the return value of mkdir(), I think you should just leave that out, as a missing directory is only one possible failure scenario here and probably the least concerning (i.e., I'd be more worried about "directory exists but is not owned by the user").
There's also a superfluous 'return dir' here - ditch the one within the if block?
Looks good to me.
--
https://code.launchpad.net/~jamesodhunt/upstart/add-session-file/+merge/144881
Your team Upstart Reviewers is subscribed to branch lp:upstart.
More information about the upstart-devel
mailing list