[Merge] lp:~jamesodhunt/upstart/setenv+getenv into lp:upstart

Colin Watson cjwatson at canonical.com
Mon Jan 21 14:22:21 UTC 2013


Review: Approve

list-env: Hmm, I kind of feel like environment variables are identifiers rather than words, and hence should be subject to C-locale sorting rather than anything more sophisticated.  It's not desperately important, though.

Security: Agreed that we should limit the scope for damage here.  Why not just forbid set-env etc. for PID 1, or when not running as a session init, or a similar condition?  After all, we haven't needed this for PID 1 so far.  Of course, this would mean that the job communication gadget you describe wouldn't work; but you could perhaps allow job-local settings reasonably safely.

Could you refactor the security checks in control_*_env into a common function?  I realise that this might involve slightly less informative messages; on the other hand a general verb like "modify" would probably do in place of "set", "reset", etc., and would be less work for translators.

Looks good to me once these bits are tidied up.
-- 
https://code.launchpad.net/~jamesodhunt/upstart/setenv+getenv/+merge/142939
Your team Upstart Reviewers is requested to review the proposed merge of lp:~jamesodhunt/upstart/setenv+getenv into lp:upstart.



More information about the upstart-devel mailing list