Comment 12 for bug 183729

Hi Petr,

I received the assignment, thanks very much! Sorry, I haven't dropped this; here's a quick review of the code, and thoughts on merging it with 0.6

0.6.x have a util/utmp.c already for reading and writing runlevel and reboot records, it feels like these process records should go in there as well - but then that'd mean having utmp.c shared between init/ and util/ and there's no directory for that yet (it's not appropriate to go into libnih)

Why the __GLIBC__ check? Unless I'm mistaken, the gettimeofday() and assign would work in either case. In general I've tried to avoid #ifdef in the Upstart code. Should use the utmpx.h functions rather than utmp.h?

Thoughts on the utmp stanza, firstly we should check the length of this at parse time rather than silently truncating. Also given instance jobs we probably want to expand environment variables here so you can do:

    utmp $ID

For things like a single getty job, that wouldn't be sufficient since you'd want the ids to match the getty ids. That'd mean supporting shell-style replacements in expansion (which we don't do yet)

    utmp ${TTY#tty}

Should init not always set DEAD_PROCESS for any pid it's supervised, whether or not it created the utmp entry in the first place? Problem there of course is that we don't know the ut_id of them I guess, so we'd have to iterate the entire utmp file every time?

paths.h defines a WTMP macro, this shouldn't be necessary since _PATH_WTMP is already defined in utmp.h

(Arguably a few of those things in paths.h should go away and be replaced by things from <paths.h> like _PATH_CONSOLE, _PATH_DEVNULL, _PATH_BSHELL, etc.)

Missing tests, should check for a utmp record being written, failing to write, etc. (possible by calling utmpname() in the test) - it should be ok for wtmp to fail at any time, so the test is safe. Should check for the entries being cleared too.

One thing that the manpage isn't quite clear about is what happens when you create an INIT_PROCESS entry but search for a USER_PROCESS entry to delete, as you've done there - I think this will still find it, but a test would help us be sure.