Review: Needs Fixing
Hi Dmitrijs,
* Changelog: Need extra indent for 'priority' and 'correctly'.
* init/conf.c:
- Might just as well add ',2013' to copyright now :)
- conf_to_job_name():
- Typo: 'ConfigSource' should be 'ConfSource'.
- Typo: 'from then' should be 'from the'.
- You could drop the braces for the final if/else test, but they
were there originally I guess :)
- conf_get_best_override():
- Typo: '& finds the' should be '& find the'.
- Typo: 'for override file' should be 'for override files'.
- Typo: 'ConfigSource' should be 'ConfSource'.
- @name and @last_source are not asserted.
- Using lstat(2) seems correct since we don't support symlinks on
purpose, but conf_source_reload_file() is still using stat(2) so
there is a discrepancy.
- conf_load_path_with_override():
- Need space around '=' in initial assignments.
- 'if (override_path)' at line 818 technically redundant since we already
know it
cannot be NULL.
- conf_create_modify_handler():
- Missing check on return from nih_sprintf().
- conf_delete_handler():
- Typo: 'overide' rather than 'override'.
Regarding your comments on the lstat issue, we need to keep an eye on this. As
you say, the impact should not be great, but it would still be worth performing
some tests on a slow system to see if the new code makes any appreciable
difference to performance.
--
https://code.launchpad.net/~xnox/upstart/overrides/+merge/141914
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
