Review: Needs Fixing

Hi Steve,

Good catch! The patch seems to have a stray 'needs_devtmpfs' at line 304:

301         if (stat ("/dev/ptmx", &statbuf) < 0 || !S_ISCHR(statbuf.st_mode) 
302             || major(statbuf.st_dev) != 5 || minor(statbuf.st_dev) != 2) 
303             needs_devtmpfs = 1;            
304         if (needs_devtmpfs
305             || stat("/dev/pts", &statbuf) < 0 || !S_ISDIR(statbuf.st_mode))
306             needs_devtmpfs = 1;

Also, I noticed (gcc didn't ;-) that the mknod parameters have been 
inadvertently transposed. To be safe we should also specify the permissions for 
the device nodes we're creating (and possibly explicitly set umask).

Finally, I think we need a slightly more holistic view on 
Upstart-in-initramfs-less-environments since we cannot assume any devices 
exist, including /dev/null, /dev/console, /dev/kmsg, /dev/tty. On Ubuntu, I 
don't think we'd see an issue since when a system is installed from the live 
media, some devices already exist. However, we cannot assume that for all 
systems.


-- 
https://code.launchpad.net/~vorlon/upstart/lp.980917/+merge/117985
Your team Upstart Reviewers is requested to review the proposed merge of 
lp:~vorlon/upstart/lp.980917 into 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