Review: Needs Fixing
+ p = strchr (contents, '=');
+ if (! p)
+ continue;
+
+ /* Invalid contents */
+ if (strncmp (contents, "UPSTART_SESSION", (p - contents)))
+ continue;
+
+ session = p + 1;
+
given that the format of this file is a fixed prefix, "UPSTART_SESSION=", I
would skip the strchr() here and just use strncmp() to check the prefix. E.g.:
/* Invalid contents */
if (strncmp (contents, "UPSTART_SESSION=", 16))
continue;
session = contents + 16;
Maybe this next check should be moved earlier, before the file I/O?:
+ if (kill (pid, 0)) {
+ nih_info ("%s: %s", _("Ignoring stale session file"),
path);
+ continue;
+ }
Those are both minor style comments and shouldn't block the merge; this next
issue though will cause problems for users trying to build upstart on their
desktop system without the use of a chroot:
+ TEST_FEATURE ("with no instances");
The 'initctl list-sessions' test appears to assume that XDG_RUNTIME_DIR is not
set in the environment, and does not explicitly unset it. This is not a safe
assumption; it should be explicitly unset in the test.
--
https://code.launchpad.net/~jamesodhunt/upstart/initctl-list-sessions/+merge/145325
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
