[Merge] lp:~xnox/upstart/upstart-user-mode into lp:upstart

James Hunt james.hunt at canonical.com
Tue Dec 18 10:35:54 UTC 2012


Review: Needs Fixing

* init/xdg.c:
  - Build fails (unused variables) if you compile like this:
    http://upstart.ubuntu.com/cookbook/#setting-up-an-upstart-development-environment
  - get_home_subdir():
    - Suffix argument should be const.
    - Minor layout issues (for example, "getenv ("
      rather than "getenv(", "(dir " rather than "( dir ", etc).
    - Needs a "Returns: newly-allocated path, or NULL on error.
  - xdg_get_config_home():
    - Needs "Returns:" comment.
  - xdg_get_config_dirs():
    - Needs "Returns:" comment.
    - That 2nd env_path assignment is very C99-ish. I'd rather
      nih_strdup() if the initial assignment fails, which would also be
      consistent with xdg_get_config_home().
  - get_user_upstart_dirs():
    - Incorrect function name in header ("xdg_get_dirs").
    - Needs "Returns:" comment.
    - Never frees 'dirs' array.
    - The header should document that the returned array is in some sort
      of order (most important directory first?)
  - main():
    - Since we've still got 'conf_dir', we need to document (somewhere)
      the relative order these directories will be traversed in
      (conf_dir versus the return value of get_user_upstart_dirs()).


-- 
https://code.launchpad.net/~xnox/upstart/upstart-user-mode/+merge/140258
Your team Upstart Reviewers is subscribed to branch lp:upstart.



More information about the upstart-devel mailing list