Code review comment for lp:~jamesodhunt/ubuntu/precise/upstart/1.4

+ /* Save old handler as grantpt disallows child handler
+ * to be in effect
+ */
+ if (sigaction (SIGCHLD, NULL, &act) < 0) {
+ close (pty_master);
+ job_process_error_abort (fds[1], JOB_PROCESS_ERROR_OPENPT_MASTER, 0);
+ }

You can probably make this code (and similar following) easier to read by dropping the explicit close; after all, you're about to exit via job_process_error_abort anyway. (I had to explicitly check that you didn't need to clean up anything else, such as the SIGCHLD handler; I think it would be clearer to either clean up everything or nothing, and all other things being equal then shortest code should probably win.)

This code only queries the current setting of the SIGCHLD handler, but doesn't actually uninstall the handler; judging from your comment and the documentation of grantpt(3), this isn't what you meant. The reason for the restriction on grantpt is that it forks a helper process and waits for it. I think, therefore, that you should call 'sigaction (SIGCHLD, SIG_IGN, &act)'.

+ errno = 0;
+ struct passwd *pwd = getpwnam (class->setuid);

I think it's standard in Upstart to have declarations at the top of a block (although it's true that C99 allows this). So local style would probably be:

  struct passwd *pwd;

  errno = 0;
  pwd = getpwnam (class->setuid);

... and likewise for the group name.

The log file writing code looks like it has the right kind of structure, although I must admit I haven't sat down to exhaustively verify it. As a minor point, I did notice that you aren't consistently casting wlen to (size_t) when passing it to nih_io_buffer_shrink; it should be either always casted or never casted. (I realise that len is size_t already.)

+/**
+ * NihOption setter function to handle selection of default console
+ * type.
+ *
+ * Returns 1 on success, -1 on invalid console type.
+ **/
+static int
+console_type_setter (NihOption *option, const char *arg)

Aren't NihOptionSetter functions supposed to return 0 on success rather than 1? nih_option_handle and nih_option_handle_arg pass through their return value, and both those functions document that they return 0 on success.

 .SH SYNOPSIS
+.TP
 .B /etc/init/
-
+Default location of system job configuration files.
+.\"
+.TP
 .B $HOME/.init/
+Default location of user job configuration files.
+.RE
 .\"
 .SH DESCRIPTION

I feel a bit uncomfortable at seeing .RE without a preceding .RS. I'd just omit that macro; the following .SH should restore the left margin for you.

+If this stanza is unspecified, the job will run as root in the case of
+system jobs, and as the user in the case of User Jobs.
+
+Note that System jobs using the setuid stanza are still system jobs,
+and can not be controlled by an unprivileged user, even if the setuid
+stanza specifies that user.

This description and the description of setgid both seem to have Random capitalisation Syndrome. :-)