Merge lp:~jamesodhunt/upstart/bug-1083723 into lp:~upstart-devel/upstart/1.6
Proposed by
James Hunt
Status: | Merged |
---|---|
Merged at revision: | 1393 |
Proposed branch: | lp:~jamesodhunt/upstart/bug-1083723 |
Merge into: | lp:~upstart-devel/upstart/1.6 |
Diff against target: |
258 lines (+160/-6) 6 files modified
dbus/com.ubuntu.Upstart.xml (+2/-0) init/control.c (+55/-0) init/control.h (+3/-0) util/Makefile.am (+2/-1) util/man/telinit.8 (+7/-1) util/telinit.c (+91/-4) |
To merge this branch: | bzr merge lp:~jamesodhunt/upstart/bug-1083723 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Steve Langasek | Needs Fixing | ||
Review via email:
|
To post a comment you must log in.
+ /* Disallow users from restarting Upstart, unless they happen to
+ * own this process (which they may do in the test scenario and
+ * when running Upstart as a non-privileged user).
+ */
+ if (session && session->user && session->user != uid) {
Same comment as for https:/ /code.launchpad .net/~jamesodhu nt/upstart/ bug-1079715/ +merge/ 135388: this should be 'session && session->user != uid', and not assume that all users should be allowed to call this method for a non-null session with user==0.
+ if (init_is_upstart ()) {
+ ret = restart_upstart ();
+ }
Please get rid of the init_is_upstart() check (and function); now that restarting is done over dbus, we shouldn't open a separate connection first to check that init is upstart. I think it's reasonable and appropriate that if someone calls 'telinit u' against a non-upstart init, and as a result init does not re-exec itself, telinit should exit non-zero instead of silently failing to take the requested action. Note that this is not an issue for the library upgrade case, because the library packages sensibly do:
telinit u 2>/dev/null || true
If there is some reason I'm missing that it's important to return 0 in the case of a non-upstart init, please at least don't call upstart_open() twice, but instead save the result so we're not setting up and tearing down the dbus connection twice.
Otherwise, this looks good.