Merge lp:~jamesodhunt/upstart/handle-no-home-var into lp:upstart

Proposed by James Hunt
Status: Rejected
Rejected by: Dimitri John Ledkov
Proposed branch: lp:~jamesodhunt/upstart/handle-no-home-var
Merge into: lp:upstart
Diff against target: 110 lines (+46/-7)
3 files modified
ChangeLog (+7/-0)
init/tests/test_xdg.c (+21/-3)
init/xdg.c (+18/-4)
To merge this branch: bzr merge lp:~jamesodhunt/upstart/handle-no-home-var
Reviewer Review Type Date Requested Status
Dimitri John Ledkov Disapprove
Review via email: mp+210621@code.launchpad.net

Description of the change

Currently, if $HOME is unset and upstart is run as a Session Init, it will spin at 100% CPU,due to the following in main():

592 dirs = NIH_MUST (get_user_upstart_dirs ());

We could just change that logic, but a better approach is to query the password database to determine the users home directory as that is a more reliable method anyway.

To post a comment you must log in.
Revision history for this message
Dimitri John Ledkov (xnox) wrote :

user-session init shouldn't be started without $HOME set in the environment. getpwent call can block on looking up NIS/ldap, and the expectation is that by the time upstart --user is run $HOME is properly available.

upstart as PID 2 shoudn't be running with --user flag, as we don't want /root/.config/upstart config dir nor logs in /root/.cache/upstart. We want it to read /etc/init and log into /var/log/upstart.

review: Disapprove

Unmerged revisions

1612. By James Hunt

* init/xdg.c: get_home_subdir() Try harder to establish users home
  directory to handle environments where $HOME may not be set.
* init/tests/test_xdg.c: Updated tests based on new safer behaviour of
  get_home_subdir().

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'ChangeLog'
--- ChangeLog 2014-03-11 14:44:56 +0000
+++ ChangeLog 2014-03-12 14:34:54 +0000
@@ -1,3 +1,10 @@
12014-03-12 James Hunt <james.hunt@ubuntu.com>
2
3 * init/xdg.c: get_home_subdir() Try harder to establish users home
4 directory to handle environments where $HOME may not be set.
5 * init/tests/test_xdg.c: Updated tests based on new safer behaviour of
6 get_home_subdir().
7
12014-03-11 James Hunt <james.hunt@ubuntu.com>82014-03-11 James Hunt <james.hunt@ubuntu.com>
29
3 * NEWS: Release 1.12.110 * NEWS: Release 1.12.1
411
=== modified file 'init/tests/test_xdg.c'
--- init/tests/test_xdg.c 2013-11-12 12:17:30 +0000
+++ init/tests/test_xdg.c 2014-03-12 14:34:54 +0000
@@ -53,11 +53,19 @@
53 TEST_FUNCTION ("get_home_subdir");53 TEST_FUNCTION ("get_home_subdir");
5454
55 TEST_FEATURE ("with HOME not set");55 TEST_FEATURE ("with HOME not set");
56 /* It should be possible to establish the home directory (via
57 * the password database) even if $HOME is not set.
58 */
56 TEST_EQ (unsetenv ("HOME"), 0);59 TEST_EQ (unsetenv ("HOME"), 0);
5760
58 TEST_ALLOC_FAIL {61 TEST_ALLOC_FAIL {
59 dir = get_home_subdir ("test", FALSE);62 dir = get_home_subdir ("test", FALSE);
60 TEST_EQ_P (dir, NULL);63 if (test_alloc_failed) {
64 TEST_EQ_P (dir, NULL);
65 } else {
66 TEST_NE_P (dir, NULL);
67 nih_free (dir);
68 }
61 }69 }
6270
63 TEST_FEATURE ("with HOME set");71 TEST_FEATURE ("with HOME set");
@@ -212,7 +220,12 @@
212 TEST_ALLOC_FAIL {220 TEST_ALLOC_FAIL {
213 outname = NULL;221 outname = NULL;
214 outname = function ();222 outname = function ();
215 TEST_EQ_P (outname, NULL);223 if (test_alloc_failed) {
224 TEST_EQ_P (outname, NULL);
225 } else {
226 TEST_NE_P (outname, NULL);
227 nih_free (outname);
228 }
216 }229 }
217230
218 TEST_FEATURE ("without HOME set and without environment override");231 TEST_FEATURE ("without HOME set and without environment override");
@@ -220,7 +233,12 @@
220 TEST_ALLOC_FAIL {233 TEST_ALLOC_FAIL {
221 outname = NULL;234 outname = NULL;
222 outname = function ();235 outname = function ();
223 TEST_EQ_P (outname, NULL);236 if (test_alloc_failed) {
237 TEST_EQ_P (outname, NULL);
238 } else {
239 TEST_NE_P (outname, NULL);
240 nih_free (outname);
241 }
224 }242 }
225}243}
226244
227245
=== modified file 'init/xdg.c'
--- init/xdg.c 2013-11-16 12:42:57 +0000
+++ init/xdg.c 2014-03-12 14:34:54 +0000
@@ -26,6 +26,8 @@
26#include <stdlib.h>26#include <stdlib.h>
27#include <sys/stat.h>27#include <sys/stat.h>
28#include <sys/types.h>28#include <sys/types.h>
29#include <sys/types.h>
30#include <pwd.h>
2931
30#include <nih/alloc.h>32#include <nih/alloc.h>
31#include <nih/logging.h>33#include <nih/logging.h>
@@ -85,13 +87,25 @@
85char *87char *
86get_home_subdir (const char *suffix, int create)88get_home_subdir (const char *suffix, int create)
87{89{
88 char *env;90 char *env;
91 struct passwd *pw;
8992
90 env = getenv ("HOME");93 env = getenv ("HOME");
91 if (! env)94 if (env)
92 return NULL;95 goto out;
9396
97 pw = getpwent ();
98 if (! pw)
99 return NULL;
100
101 if (! pw->pw_dir)
102 return NULL;
103
104 env = pw->pw_dir;
105
106out:
94 return get_subdir (env, suffix, create);107 return get_subdir (env, suffix, create);
108
95}109}
96110
97/**111/**

Subscribers

People subscribed via source and target branches