[Merge] lp:~jamesodhunt/upstart/add-session-file into lp:upstart
Dmitrijs Ledkovs
launchpad at surgut.co.uk
Tue Jan 29 09:27:22 UTC 2013
> 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.
>
It's a check for absolute path.
>
> +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").
>
Sounds like we shouldn't try to create the top level one and we are only ok to create sub-dirs inside runtimedir.
> There's also a superfluous 'return dir' here - ditch the one within the if
> block?
>
If the path is not absolute, it should return NULL ?!
> 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