James Hunt has proposed merging lp:~jamesodhunt/upstart/bugs-1235649+1203595 into lp:upstart.
Requested reviews: Upstart Reviewers (upstart-reviewers) Related bugs: Bug #1203595 in upstart : "session init, not available on dbus?" https://bugs.launchpad.net/upstart/+bug/1203595 Bug #1235649 in upstart : "uevent spam causes libdbus client code in session upstart to consume massive amounts of memory on Ubuntu Touch" https://bugs.launchpad.net/upstart/+bug/1235649 For more details, see: https://code.launchpad.net/~jamesodhunt/upstart/bugs-1235649+1203595/+merge/191852 Make a Session Init connect to the D-Bus session bus as well as the private socket. -- https://code.launchpad.net/~jamesodhunt/upstart/bugs-1235649+1203595/+merge/191852 Your team Upstart Reviewers is requested to review the proposed merge of lp:~jamesodhunt/upstart/bugs-1235649+1203595 into lp:upstart.
=== modified file 'ChangeLog' --- ChangeLog 2013-10-04 21:34:25 +0000 +++ ChangeLog 2013-10-22 10:15:27 +0000 @@ -1,4 +1,51 @@ -2013-01-04 Steve Langasek <[email protected] +2013-10-22 James Hunt <[email protected]> + + * init/control.c: control_bus_open(): Don't call nih_dbus_bus() if + DBUS_SESSION_BUS_ADDRESS is not set to avoid D-bus auto-launching a + dbus-daemon. + * init/environ.c: Comments. + * init/job_class.c: + - job_class_environment_init(): Superior check on whether job_environ + is not empty. + - job_class_environment_reset(): Only reset job_environ if not NULL + already. + - job_class_environment_set(): Set variable in Upstarts environment + too (required to allow Upstart to be aware of the D-Bus session bus + address when the dbus-daemon is available). + - job_class_environment_unset(): Unset variable from Upstarts + environment, but only if it is not a default variable. + * init/job_process.c: Formatting. + * init/test_control.c: Updated strings used by tests which check error + messages to include 'D-Bus'. + * init/test_environ.c: + - test_add(): New test: + - "using bare word with no corresponding variable set in environment" + - test_remove(): New function ("the missing test") containing 8 new tests: + - "remove name=value pair with empty table" + - "remove bare name with empty table" + - "remove name=value from table of size 1" + - "remove bare name from table of size 1" + - "remove first name=value entry from table of size 2" + - "remove first bare name entry from table of size 2" + - "remove last name=value entry from table of size 2" + - "remove last bare name entry from table of size 2" + * test/test_util_common.c: + - Formatting. + - get_initctl(): Added environment checks. + * util/initctl.c: + - Formatting. + - Removed testing comment from option text for '--session'. + * util/man/initctl.8: Removed testing comment for '--session'. + * util/tests/test_initctl.c: + - test_session_init(): New test that checks the Session Init now + connects to the D-Bus session bus. + +2013-10-18 James Hunt <[email protected]> + + * Connect to the D-Bus session bus as well as using a private socket + when running as a Session Init (LP: #1203595, #1235649). + +2013-10-04 Steve Langasek <[email protected] * extra/upstart-local-bridge.c: use SOCKET_PATH in our event environment, instead of clobbering PATH. (LP: #1235480) === modified file 'init/control.c' --- init/control.c 2013-04-22 10:30:09 +0000 +++ init/control.c 2013-10-22 10:15:27 +0000 @@ -81,11 +81,19 @@ * * If TRUE, connect to the D-Bus session bus rather than the system bus. * - * Used for testing. + * Used for testing to simulate (as far as possible) a system-like init + * when running as a non-priv user. **/ int use_session_bus = FALSE; /** + * dbus_bus_type: + * + * Type of D-Bus bus to connect to. + **/ +DBusBusType dbus_bus_type; + +/** * control_server_address: * * Address on which the control server may be reached. @@ -258,16 +266,35 @@ control_init (); - control_handle_bus_type (); - - /* Connect to the D-Bus System Bus and hook everything up into + dbus_bus_type = control_get_bus_type (); + + /* Avoid D-Bus attempting to autolaunch a dbus-daemon since: + * + * - the behaviour is undesirable: launching a dbus-daemon + * should be handled by a job. + * + * - if autolaunch fails (for example if DISPLAY is not set, or + * dbus-uuidgen has not been run (or the generated file is + * unreadable)), libdbus may call abort(), depending on how + * libdbus has been built. + */ + if (dbus_bus_type == DBUS_BUS_SESSION && ! getenv ("DBUS_SESSION_BUS_ADDRESS")) { + nih_dbus_error_raise ("dbus session bus error", + "DBUS_SESSION_BUS_ADDRESS not set"); + return -1; + } + + /* Connect to the appropriate D-Bus bus and hook everything up into * our own main loop automatically. */ - conn = nih_dbus_bus (use_session_bus ? DBUS_BUS_SESSION : DBUS_BUS_SYSTEM, - control_disconnected); + conn = nih_dbus_bus (dbus_bus_type, control_disconnected); if (! conn) return -1; + nih_debug ("Connected to D-Bus %s bus", + dbus_bus_type == DBUS_BUS_SESSION + ? "session" : "system"); + /* Register objects on the bus. */ control_register_all (conn); @@ -345,7 +372,9 @@ dbus_error_init (&error); - nih_warn (_("Disconnected from system bus")); + nih_warn (_("Disconnected from D-Bus %s bus"), + dbus_bus_type == DBUS_BUS_SESSION + ? "session" : "system"); control_bus = NULL; } @@ -817,19 +846,23 @@ } /** - * control_handle_bus_type: + * control_get_bus_type: * * Determine D-Bus bus type to connect to. + * + * Returns: Type of D-Bus bus to connect to. **/ -void -control_handle_bus_type (void) +DBusBusType +control_get_bus_type (void) { if (getenv (USE_SESSION_BUS_ENV)) use_session_bus = TRUE; - if (use_session_bus) - nih_debug ("Using session bus"); + return (use_session_bus || user_mode) + ? DBUS_BUS_SESSION + : DBUS_BUS_SYSTEM; } + /** * control_notify_disk_writeable: * @data: not used, === modified file 'init/control.h' --- init/control.h 2013-05-08 16:21:08 +0000 +++ init/control.h 2013-10-22 10:15:27 +0000 @@ -134,7 +134,11 @@ const char *log_priority) __attribute__ ((warn_unused_result)); -void control_handle_bus_type (void); +DBusBusType control_get_bus_type (void) + __attribute__ ((warn_unused_result)); + +const char *control_get_bus_name (void) + __attribute__ ((warn_unused_result)); void control_prepare_reexec (void); === modified file 'init/environ.c' --- init/environ.c 2013-01-08 15:57:31 +0000 +++ init/environ.c 2013-10-22 10:15:27 +0000 @@ -202,6 +202,10 @@ for (e = *env; e && *e; e++) { keylen = strcspn (*e, "="); + /* Found @str in the existing environment (either as a + * name=value pair, or a bare name), so don't copy it to + * the new environment. + */ if (! strncmp (str, *e, keylen)) continue; === modified file 'init/job_class.c' --- init/job_class.c 2013-07-22 00:15:43 +0000 +++ init/job_class.c 2013-10-22 10:15:27 +0000 @@ -89,6 +89,14 @@ NihHash *job_classes = NULL; /** + * default_environ: + * + * Minimal set of environment variables all jobs are provided with by + * default. + */ +static char * const default_environ[] = { JOB_DEFAULT_ENVIRONMENT, NULL }; + +/** * job_environ: * * Array of environment variables that will be set in the jobs @@ -116,8 +124,6 @@ void job_class_environment_init (void) { - char * const default_environ[] = { JOB_DEFAULT_ENVIRONMENT, NULL }; - if (job_environ) return; @@ -125,7 +131,7 @@ NIH_MUST (environ_append (&job_environ, NULL, 0, TRUE, default_environ)); if (user_mode && ! no_inherit_env) - NIH_MUST(environ_append (&job_environ, NULL, 0, TRUE, environ)); + NIH_MUST (environ_append (&job_environ, NULL, 0, TRUE, environ)); } /** @@ -138,10 +144,10 @@ void job_class_environment_reset (void) { - if (job_environ) + if (job_environ) { nih_free (job_environ); - - job_environ = NULL; + job_environ = NULL; + } job_class_environment_init (); } @@ -160,9 +166,31 @@ int job_class_environment_set (const char *var, int replace) { + nih_local char *name = NULL; + char *value; + nih_assert (var); nih_assert (job_environ); + name = nih_strdup (NULL, var); + if (! name) + return -1; + + value = strchr (name, '='); + nih_assert (value); + *value = '\0'; + + /* Jump over delimiter */ + value++; + + /* Set variable in Upstarts environment. + * + * This isn't necessary for jobs (due to job_environ), but is + * required to allow the Session Init to connect to the D-Bus + * session bus. + */ + setenv (name, value, replace); + if (! environ_add (&job_environ, NULL, NULL, replace, var)) return -1; @@ -193,9 +221,20 @@ int job_class_environment_unset (const char *name) { + char * const *e; + int keep = FALSE; + nih_assert (name); nih_assert (job_environ); + /* Determine if @name is a default variable */ + for (e = default_environ; e && *e; e++) { + if (! strcmp (*e, name)) { + keep = TRUE; + break; + } + } + if (! environ_remove (&job_environ, NULL, NULL, name)) return -1; @@ -211,6 +250,17 @@ } } + if (! keep) { + /* Remove variable from Upstarts environment, but only if it is + * not part of the default environment; if we did this, + * it would not be possible to reset the job environment + * since adding a variable by name would *not* be added + * to the job environment table since the environment + * lookup would fail. + */ + unsetenv (name); + } + return 0; } === modified file 'init/job_process.c' --- init/job_process.c 2013-10-03 14:43:24 +0000 +++ init/job_process.c 2013-10-22 10:15:27 +0000 @@ -278,7 +278,7 @@ env = NIH_MUST (nih_str_array_new (NULL)); if (job->env) - NIH_MUST(environ_append (&env, NULL, &envc, TRUE, job->env)); + NIH_MUST (environ_append (&env, NULL, &envc, TRUE, job->env)); if (job->stop_env && ((process == PROCESS_PRE_STOP) === modified file 'init/main.c' --- init/main.c 2013-07-31 09:28:48 +0000 +++ init/main.c 2013-10-22 10:15:27 +0000 @@ -128,7 +128,7 @@ extern int default_console; extern int write_state_file; extern char *log_dir; - +extern DBusBusType dbus_bus_type; /** * options: @@ -213,7 +213,8 @@ if (disable_job_logging) nih_debug ("Job logging disabled"); - control_handle_bus_type (); + if (getenv (USE_SESSION_BUS_ENV)) + use_session_bus = TRUE; if (! user_mode) no_inherit_env = TRUE; @@ -933,14 +934,20 @@ NihSignal *signal) { if (! control_bus) { - nih_info (_("Reconnecting to system bus")); + char *dbus_bus_name; + + dbus_bus_name = dbus_bus_type == DBUS_BUS_SESSION + ? "session" : "system"; + + nih_info (_("Reconnecting to D-Bus %s bus"), + dbus_bus_name); if (control_bus_open () < 0) { NihError *err; err = nih_error_get (); - nih_warn ("%s: %s", _("Unable to connect to the system bus"), - err->message); + nih_warn (_("Unable to connect to the D-Bus %s bus: %s"), + dbus_bus_name, err->message); nih_free (err); } } === modified file 'init/tests/test_control.c' --- init/tests/test_control.c 2012-09-09 21:27:24 +0000 +++ init/tests/test_control.c 2013-10-22 10:15:27 +0000 @@ -821,7 +821,7 @@ TEST_LIST_EMPTY (control_conns); - TEST_FILE_EQ (output, "test: Disconnected from system bus\n"); + TEST_FILE_EQ (output, "test: Disconnected from D-Bus system bus\n"); TEST_FILE_END (output); TEST_FILE_RESET (output); @@ -879,7 +879,7 @@ TEST_LIST_EMPTY (control_conns); - TEST_FILE_EQ (output, "test: Disconnected from system bus\n"); + TEST_FILE_EQ (output, "test: Disconnected from D-Bus system bus\n"); TEST_FILE_END (output); TEST_FILE_RESET (output); === modified file 'init/tests/test_environ.c' --- init/tests/test_environ.c 2011-06-06 17:05:11 +0000 +++ init/tests/test_environ.c 2013-10-22 10:15:27 +0000 @@ -572,6 +572,462 @@ } unsetenv ("BAR"); + + /* Check that attempting to add a variable by name fails if + * there is no corresponding environment variable set. + */ + TEST_FEATURE ("using bare word with no corresponding variable set in environment"); + + /* Ensure variable not set initially */ + TEST_EQ_P (getenv ("UPSTART_TEST_VARIABLE"), NULL); + + TEST_ALLOC_FAIL { + TEST_ALLOC_SAFE { + len = 0; + env = nih_str_array_new (NULL); + assert (nih_str_array_add (&env, NULL, &len, + "FOO=BAR")); + } + + ret = environ_add (&env, NULL, &len, FALSE, "UPSTART_TEST_VARIABLE"); + + if (test_alloc_failed) { + TEST_EQ_P (ret, NULL); + + TEST_EQ (len, 1); + TEST_EQ_STR (env[0], "FOO=BAR"); + TEST_EQ_P (env[1], NULL); + + nih_free (env); + continue; + } + + /* XXX: Attempting to add an unset variable results in + * no change to the table (it is not an error!) + */ + TEST_EQ_P (ret, env); + + TEST_EQ (len, 1); + TEST_EQ_STR (env[0], "FOO=BAR"); + TEST_EQ_P (env[1], NULL); + + nih_free (env); + } +} + +void +test_remove (void) +{ + char **env = NULL, **ret; + size_t len = 0; + + TEST_FUNCTION ("environ_remove"); + + TEST_FEATURE ("remove name=value pair with empty table"); + TEST_ALLOC_FAIL { + TEST_ALLOC_SAFE { + len = 0; + env = nih_str_array_new (NULL); + } + + ret = environ_remove (&env, NULL, &len, "FOO=BAR"); + + if (test_alloc_failed) { + TEST_EQ_P (ret, NULL); + + TEST_EQ (len, 0); + TEST_EQ_P (env[0], NULL); + + nih_free (env); + continue; + } + + TEST_EQ_P (ret, NULL); + TEST_EQ (len, 0); + TEST_EQ_P (env[0], NULL); + + nih_free (env); + } + + TEST_FEATURE ("remove bare name with empty table"); + TEST_ALLOC_FAIL { + TEST_ALLOC_SAFE { + len = 0; + env = nih_str_array_new (NULL); + } + + ret = environ_remove (&env, NULL, &len, "FOO"); + + if (test_alloc_failed) { + TEST_EQ_P (ret, NULL); + + TEST_EQ (len, 0); + TEST_EQ_P (env[0], NULL); + + nih_free (env); + continue; + } + + TEST_EQ_P (ret, NULL); + TEST_EQ (len, 0); + TEST_EQ_P (env[0], NULL); + + nih_free (env); + } + + TEST_FEATURE ("remove name=value from table of size 1"); + TEST_ALLOC_FAIL { + TEST_ALLOC_SAFE { + len = 0; + env = nih_str_array_new (NULL); + + ret = environ_add (&env, NULL, &len, TRUE, "FOO=BAR"); + TEST_NE_P (ret, NULL); + + TEST_EQ (len, 1); + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "FOO=BAR"); + TEST_EQ_P (env[1], NULL); + } + + ret = environ_remove (&env, NULL, &len, "FOO=BAR"); + + if (test_alloc_failed) { + TEST_EQ_P (ret, NULL); + + TEST_EQ (len, 1); + + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + + TEST_NE_P (env[0], NULL); + TEST_EQ_STR (env[0], "FOO=BAR"); + + TEST_EQ_P (env[1], NULL); + + nih_free (env); + continue; + } + + TEST_NE_P (ret, NULL); + TEST_EQ (len, 0); + TEST_EQ_P (env[0], NULL); + + nih_free (env); + } + + TEST_FEATURE ("remove bare name from table of size 1"); + TEST_ALLOC_FAIL { + TEST_ALLOC_SAFE { + len = 0; + env = nih_str_array_new (NULL); + + ret = environ_add (&env, NULL, &len, TRUE, "FOO=BAR"); + TEST_NE_P (ret, NULL); + + TEST_EQ (len, 1); + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "FOO=BAR"); + TEST_EQ_P (env[1], NULL); + } + + ret = environ_remove (&env, NULL, &len, "FOO"); + + if (test_alloc_failed) { + TEST_EQ_P (ret, NULL); + + TEST_EQ (len, 1); + + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + + TEST_NE_P (env[0], NULL); + TEST_EQ_STR (env[0], "FOO=BAR"); + + TEST_EQ_P (env[1], NULL); + + nih_free (env); + continue; + } + + TEST_NE_P (ret, NULL); + TEST_EQ (len, 0); + TEST_EQ_P (env[0], NULL); + + nih_free (env); + } + + TEST_FEATURE ("remove first name=value entry from table of size 2"); + TEST_ALLOC_FAIL { + TEST_ALLOC_SAFE { + len = 0; + env = nih_str_array_new (NULL); + + ret = environ_add (&env, NULL, &len, TRUE, "FOO=BAR"); + TEST_NE_P (ret, NULL); + + TEST_EQ (len, 1); + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "FOO=BAR"); + TEST_EQ_P (env[1], NULL); + + ret = environ_add (&env, NULL, &len, TRUE, "BAZ=QUX"); + TEST_NE_P (ret, NULL); + + TEST_EQ (len, 2); + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "FOO=BAR"); + + TEST_ALLOC_PARENT (env[1], env); + TEST_ALLOC_SIZE (env[1], 8); + TEST_EQ_STR (env[1], "BAZ=QUX"); + + TEST_EQ_P (env[2], NULL); + } + + /* Remove first entry added */ + ret = environ_remove (&env, NULL, &len, "FOO=BAR"); + + if (test_alloc_failed) { + TEST_EQ_P (ret, NULL); + + TEST_EQ (len, 2); + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "FOO=BAR"); + + TEST_ALLOC_PARENT (env[1], env); + TEST_ALLOC_SIZE (env[1], 8); + TEST_EQ_STR (env[1], "BAZ=QUX"); + + TEST_EQ_P (env[2], NULL); + + nih_free (env); + continue; + } + + TEST_NE_P (ret, NULL); + TEST_EQ (len, 1); + + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "BAZ=QUX"); + + TEST_EQ_P (env[1], NULL); + + nih_free (env); + } + + TEST_FEATURE ("remove first bare name entry from table of size 2"); + + /* Set a variable to allow the bare name to be expanded */ + assert0 (setenv ("UPSTART_TEST_VARIABLE", "foo", 1)); + + TEST_ALLOC_FAIL { + TEST_ALLOC_SAFE { + len = 0; + env = nih_str_array_new (NULL); + + ret = environ_add (&env, NULL, &len, TRUE, "UPSTART_TEST_VARIABLE"); + TEST_NE_P (ret, NULL); + + TEST_EQ (len, 1); + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + + /* Should have been expanded */ + TEST_EQ_STR (env[0], "UPSTART_TEST_VARIABLE=foo"); + + TEST_EQ_P (env[1], NULL); + + ret = environ_add (&env, NULL, &len, TRUE, "BAZ=QUX"); + TEST_NE_P (ret, NULL); + + TEST_EQ (len, 2); + + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "UPSTART_TEST_VARIABLE=foo"); + + TEST_ALLOC_PARENT (env[1], env); + TEST_ALLOC_SIZE (env[1], 8); + TEST_EQ_STR (env[1], "BAZ=QUX"); + + TEST_EQ_P (env[2], NULL); + } + + /* Remove first entry added */ + ret = environ_remove (&env, NULL, &len, "UPSTART_TEST_VARIABLE"); + + if (test_alloc_failed) { + TEST_EQ_P (ret, NULL); + + TEST_EQ (len, 2); + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "UPSTART_TEST_VARIABLE=foo"); + + TEST_ALLOC_PARENT (env[1], env); + TEST_ALLOC_SIZE (env[1], 8); + TEST_EQ_STR (env[1], "BAZ=QUX"); + + TEST_EQ_P (env[2], NULL); + + nih_free (env); + continue; + } + + TEST_NE_P (ret, NULL); + TEST_EQ (len, 1); + + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "BAZ=QUX"); + + TEST_EQ_P (env[1], NULL); + + nih_free (env); + } + + assert0 (unsetenv ("UPSTART_TEST_VARIABLE")); + + TEST_FEATURE ("remove last name=value entry from table of size 2"); + TEST_ALLOC_FAIL { + TEST_ALLOC_SAFE { + len = 0; + env = nih_str_array_new (NULL); + + ret = environ_add (&env, NULL, &len, TRUE, "FOO=BAR"); + TEST_NE_P (ret, NULL); + + TEST_EQ (len, 1); + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "FOO=BAR"); + TEST_EQ_P (env[1], NULL); + + ret = environ_add (&env, NULL, &len, TRUE, "BAZ=QUX"); + TEST_NE_P (ret, NULL); + + TEST_EQ (len, 2); + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "FOO=BAR"); + + TEST_ALLOC_PARENT (env[1], env); + TEST_ALLOC_SIZE (env[1], 8); + TEST_EQ_STR (env[1], "BAZ=QUX"); + + TEST_EQ_P (env[2], NULL); + } + + /* Remove last entry added */ + ret = environ_remove (&env, NULL, &len, "BAZ=QUX"); + + if (test_alloc_failed) { + TEST_EQ_P (ret, NULL); + + TEST_EQ (len, 2); + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "FOO=BAR"); + + TEST_ALLOC_PARENT (env[1], env); + TEST_ALLOC_SIZE (env[1], 8); + TEST_EQ_STR (env[1], "BAZ=QUX"); + + TEST_EQ_P (env[2], NULL); + + nih_free (env); + continue; + } + + TEST_NE_P (ret, NULL); + TEST_EQ (len, 1); + + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "FOO=BAR"); + + TEST_EQ_P (env[1], NULL); + + nih_free (env); + } + + TEST_FEATURE ("remove last bare name entry from table of size 2"); + + /* Set a variable to allow the bare name to be expanded */ + assert0 (setenv ("UPSTART_TEST_VARIABLE", "foo", 1)); + + TEST_ALLOC_FAIL { + TEST_ALLOC_SAFE { + len = 0; + env = nih_str_array_new (NULL); + + ret = environ_add (&env, NULL, &len, TRUE, "FOO=BAR"); + TEST_NE_P (ret, NULL); + + TEST_EQ (len, 1); + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "FOO=BAR"); + TEST_EQ_P (env[1], NULL); + + ret = environ_add (&env, NULL, &len, TRUE, "UPSTART_TEST_VARIABLE"); + TEST_NE_P (ret, NULL); + + TEST_EQ (len, 2); + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "FOO=BAR"); + + TEST_ALLOC_PARENT (env[1], env); + TEST_ALLOC_SIZE (env[1], 8); + + /* Should have been expanded */ + TEST_EQ_STR (env[1], "UPSTART_TEST_VARIABLE=foo"); + + TEST_EQ_P (env[2], NULL); + } + + /* Remove last entry added */ + ret = environ_remove (&env, NULL, &len, "UPSTART_TEST_VARIABLE"); + + if (test_alloc_failed) { + TEST_EQ_P (ret, NULL); + + TEST_EQ (len, 2); + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "FOO=BAR"); + + TEST_ALLOC_PARENT (env[1], env); + TEST_ALLOC_SIZE (env[1], 8); + TEST_EQ_STR (env[1], "UPSTART_TEST_VARIABLE=foo"); + + TEST_EQ_P (env[2], NULL); + + nih_free (env); + continue; + } + + TEST_NE_P (ret, NULL); + TEST_EQ (len, 1); + + TEST_ALLOC_PARENT (env[0], env); + TEST_ALLOC_SIZE (env[0], 8); + TEST_EQ_STR (env[0], "FOO=BAR"); + + TEST_EQ_P (env[1], NULL); + + nih_free (env); + } + + assert0 (unsetenv ("UPSTART_TEST_VARIABLE")); } void @@ -1590,6 +2046,7 @@ setenv ("UPSTART_NO_SESSIONS", "1", 1); test_add (); + test_remove (); test_append (); test_set (); test_lookup (); === modified file 'test/test_util_common.c' --- test/test_util_common.c 2013-10-02 08:59:20 +0000 +++ test/test_util_common.c 2013-10-22 10:15:27 +0000 @@ -120,7 +120,7 @@ } /* TRUE to denote that Upstart is running in user session mode - * (FALSE to denote it's using the users D-Bus session bus). + * (FALSE to denote it's using a D-Bus session bus only). */ int test_user_mode = FALSE; @@ -349,6 +349,16 @@ { static char path[PATH_MAX + 1024] = { 0 }; int ret; + int env_valid; + + /* Sanity check calling environment */ + if (test_user_mode) { + env_valid = getenv ("UPSTART_SESSION") ? TRUE : FALSE; + } else { + env_valid = getenv ("DBUS_SESSION_BUS_ADDRESS") ? TRUE : FALSE; + } + + nih_assert (env_valid); ret = sprintf (path, "%s %s", get_initctl_binary (), === modified file 'util/initctl.c' --- util/initctl.c 2013-07-21 23:54:16 +0000 +++ util/initctl.c 2013-10-22 10:15:27 +0000 @@ -342,8 +342,7 @@ use_dbus = getuid () ? TRUE : FALSE; if (use_dbus >= 0 && dbus_bus_type < 0) dbus_bus_type = DBUS_BUS_SYSTEM; - } - else { + } else { if (! user_addr) { nih_error ("UPSTART_SESSION isn't set in the environment. " "Unable to locate the Upstart instance."); @@ -2827,7 +2826,7 @@ * Command-line options accepted for all arguments. **/ static NihOption options[] = { - { 0, "session", N_("use D-Bus session bus to connect to init daemon (for testing)"), + { 0, "session", N_("use D-Bus session bus to connect to init daemon"), NULL, NULL, NULL, dbus_bus_type_setter }, { 0, "system", N_("use D-Bus system bus to connect to init daemon"), NULL, NULL, NULL, dbus_bus_type_setter }, === modified file 'util/man/initctl.8' --- util/man/initctl.8 2013-05-31 15:41:20 +0000 +++ util/man/initctl.8 2013-10-22 10:15:27 +0000 @@ -47,9 +47,9 @@ .\" .TP .B \-\-session -Connect to +Connect to the .BR init (8) -daemon using the D\-Bus session bus (for testing purposes only). +daemon using the D\-Bus session bus. .\" .TP .B \-\-system === modified file 'util/tests/test_initctl.c' --- util/tests/test_initctl.c 2013-09-26 16:33:07 +0000 +++ util/tests/test_initctl.c 2013-10-22 10:15:27 +0000 @@ -16698,6 +16698,89 @@ TEST_EQ (rmdir (logdir), 0); } +void +test_session_init (void) +{ + size_t lines; + pid_t dbus_pid = 0; + pid_t upstart_pid = 0; + nih_local char *cmd = NULL; + char **output; + nih_local char *dbus_session_address = NULL; + nih_local char *upstart_session = NULL; + char *address; + + TEST_GROUP ("User Mode"); + + TEST_FEATURE ("ensure session init connects to D-Bus session bus"); + + /* Start a dbus-daemon */ + TEST_DBUS (dbus_pid); + + /* Not required */ + assert0 (unsetenv ("DBUS_SYSTEM_BUS_ADDRESS")); + + address = getenv ("DBUS_SESSION_BUS_ADDRESS"); + TEST_NE_P (address, NULL); + + dbus_session_address = nih_strdup (NULL, address); + TEST_NE_P (dbus_session_address, NULL); + + /* Stop Upstart connectng to the D-Bus session bus at + * startup (it will attempt to do so, but fail and try again on + * SIGUSR1. + */ + assert0 (unsetenv ("DBUS_SESSION_BUS_ADDRESS")); + + START_UPSTART (upstart_pid, TRUE); + + /* Save the Upstart session socket details and unset to stop + * initctl finding upstart via this route. + */ + address = getenv ("UPSTART_SESSION"); + TEST_NE_P (address, NULL); + upstart_session = nih_strdup (NULL, address); + TEST_NE_P (upstart_session, NULL); + assert0 (unsetenv ("UPSTART_SESSION")); + + /* Ensure it is not possible to query the running version via + * the D-Bus session bus. + */ + cmd = nih_sprintf (NULL, "%s --session version 2>&1", get_initctl_binary ()); + TEST_NE_P (cmd, NULL); + RUN_COMMAND (NULL, cmd, &output, &lines); + TEST_EQ (lines, 1); + TEST_STR_MATCH (output[0], "initctl: Name \"com.ubuntu.Upstart\" does not exist*"); + + /* Re-apply in the test environment */ + assert0 (setenv ("DBUS_SESSION_BUS_ADDRESS", dbus_session_address, 1)); + assert0 (setenv ("UPSTART_SESSION", upstart_session, 1)); + + /* Set the variable in Upstarts environment too */ + cmd = nih_sprintf (NULL, "%s --user set-env -g %s=%s", + get_initctl_binary (), + "DBUS_SESSION_BUS_ADDRESS", + dbus_session_address); + TEST_NE_P (cmd, NULL); + RUN_COMMAND (NULL, cmd, &output, &lines); + TEST_EQ (lines, 0); + + /* Send signal to upstart requesting it reconnect to D-Bus */ + assert0 (kill (upstart_pid, SIGUSR1)); + + /* It should now be possible to query the running version via + * the D-Bus session bus. + */ + cmd = nih_sprintf (NULL, "%s --session version 2>&1", get_initctl_binary ()); + TEST_NE_P (cmd, NULL); + RUN_COMMAND (NULL, cmd, &output, &lines); + TEST_EQ (lines, 1); + TEST_STR_MATCH (output[0], "init (upstart [0-9.][0-9.]*"); + + STOP_UPSTART (upstart_pid); + TEST_DBUS_END (dbus_pid); +} + int main (int argc, char *argv[]) @@ -16741,5 +16824,7 @@ test_notify_disk_writeable (); } + test_session_init (); + return 0; }
-- upstart-devel mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/upstart-devel
