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.

-- 
upstart-devel mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/upstart-devel

Reply via email to