Hi James,
I have uploaded another set of changes with most of the recommendations.
On 06/27/2011 01:05 PM, James Hunt wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 22/06/11 14:01, Surbhi Palande wrote:
Hi James,
Thanks a lot for the review and the suggestions.
I have incorporated most of the changes you suggested and pushed them to:
https://code.launchpad.net/~csurbhi/upstart/upstart-add-pivot-handling/
Hi Surbhi,
Thanks very much for making the changes. I think it's getting pretty close now.
Could you raise bugs
on NIH re exposing nih_dir_walk_scan() and the nih_file_read() stat() issue.
I have created bugs on launchpad - bug #802465, #802477, #802487
against libnih.
The remaining comments I have:
* init/pivot.h:
- If there is a chance that MS_MOVE will not already be defined, I'd
suggested documenting why and adding a line such as below in that #if
block:
#warning WARNING: MS_MOVE not defined
However, can't we just do the following to get all 3 of these
defines? This would be much safer should those magic numbers ever
change.
#include<linux/magic.h>
#include<linux/fs.h>
Thanks! I have added these instead.
* init/pivot.c:
- line 73: del_dir(): "while ( (" should be "while (("
- Would be good to add some nih_debug() calls to del_dir()
so you can see what files / directories are being deleted?
- is_init_ok(): Typo "exsists".
- get_init_args():
- Still hard-coding size of CMDLINE. Could break on x86_64.
- Still not using nih_file_read() rather than fopen() and fgets().
This is because of the stat returning a 0 on /proc/cmdline.
* init/conf.c: delete_conf_watches():
- NIH_LIST_FOREACH() indent wrong (and line below).
- The use of inotify_rm_watch() makes me wonder if you should raise a
bug on NIH since that functionality isn't available in watch.c.
* init/control.c: control_pivot_to_new_rootfs(): Could just remove the
'ret' variable and make last line:
return pivot_to_new_rootfs (message, rootfs, init);
* init/control.h: Formatting of control_emit_event_with_file()
prototype.
* init/events.h: As mentioned, PIVOT_FAILED_EVENT should be
"pivot-failed" for consistency with other events (see
upstart-events(7) on Ubuntu).
* util/initctl.c: pivot_action(): Doc says returns -1 on error, but it
is returning 1 on error consistently.
Also adding the license to the files. Please do let me know if that
looks ok.
Thanks a lot!
Warm Regards,
Surbhi.
--
upstart-devel mailing list
[email protected]
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/upstart-devel
