James Hunt has proposed merging lp:~jamesodhunt/upstart/bug-1269731 into lp:upstart.
Requested reviews: Upstart Reviewers (upstart-reviewers) Related bugs: Bug #1269731 in upstart (Ubuntu): "init crashed with SIGSEGV" https://bugs.launchpad.net/ubuntu/+source/upstart/+bug/1269731 For more details, see: https://code.launchpad.net/~jamesodhunt/upstart/bug-1269731/+merge/202083 -- https://code.launchpad.net/~jamesodhunt/upstart/bug-1269731/+merge/202083 Your team Upstart Reviewers is requested to review the proposed merge of lp:~jamesodhunt/upstart/bug-1269731 into lp:upstart.
=== modified file 'ChangeLog' --- ChangeLog 2014-01-15 11:51:18 +0000 +++ ChangeLog 2014-01-17 12:09:08 +0000 @@ -1,3 +1,15 @@ +2014-01-17 James Hunt <[email protected]> + + * init/conf.c: + - conf_file_serialise(): Handle ConfFile's without an + associated JobClass, resulting from an unparseable job configuration + file (LP: #1269731). + - conf_file_deserialise(): Comments. + * init/job_class.c: job_class_deserialise_all(): Comments. + * init/tests/test_conf.c: test_source_reload_file(): Add new tests: + - "Invalid .conf file does not stop ConfFile being serialised". + - "ConfFile with no JobClass can be deserialised". + 2014-01-15 James Hunt <[email protected]> * util/telinit.c: upstart_open(): Connect using private socket === modified file 'init/conf.c' --- init/conf.c 2013-11-16 12:42:57 +0000 +++ init/conf.c 2014-01-17 12:09:08 +0000 @@ -1636,6 +1636,19 @@ if (! state_set_json_int_var_from_obj (json, file, flag)) goto error; + if (! file->job) { + /* File exists on disk but contains invalid + * (unparseable) syntax, and hence no associated JobClass. + * Thus, simply encode the ConfFile without a class. + * + * Deserialisation is handled automatically since + * JobClasses are deserialised by directly iterating + * through all JobClass'es found in the JSON. Here, + * there simply won't be a JobClass to deserialise. + */ + goto out; + } + /* * Ignore the "best" JobClass associated with this ConfFile * (file->job) since it won't be serialised. @@ -1681,6 +1694,7 @@ json_object_object_add (json, "job_class", json_job_class); +out: return json; error: @@ -1717,8 +1731,8 @@ goto error; /* Note that the associated JobClass is not handled at this - * stage: it can't be the JobClasses haven't been deserialised - * yet. As such, the ConfFile->JobClass link is dealt with by + * stage: it can't be since the JobClasses haven't been deserialised + * yet. As such, the ConfFile->JobClass link is dealt with in * job_class_deserialise_all(). */ file->job = NULL; === modified file 'init/job_class.c' --- init/job_class.c 2013-11-04 09:34:51 +0000 +++ init/job_class.c 2014-01-17 12:09:08 +0000 @@ -2429,6 +2429,9 @@ if (! state_check_json_type (json_class, object)) goto error; + /* Responsible for associating a JobClass with its + * parent ConfFile. + */ class = job_class_deserialise (json_class); /* Either memory is low or -- more likely -- a JobClass === modified file 'init/tests/test_conf.c' --- init/tests/test_conf.c 2013-06-25 10:13:12 +0000 +++ init/tests/test_conf.c 2014-01-17 12:09:08 +0000 @@ -3724,14 +3724,15 @@ void test_source_reload_file (void) { - ConfSource *source; - ConfFile *file, *old_file; - FILE *f; - int ret, fd, nfds; - char dirname[PATH_MAX]; - char tmpname[PATH_MAX], filename[PATH_MAX]; - fd_set readfds, writefds, exceptfds; - NihError *err; + ConfSource *source; + ConfFile *file, *old_file; + FILE *f; + int ret, fd, nfds; + char dirname[PATH_MAX]; + char tmpname[PATH_MAX], filename[PATH_MAX]; + fd_set readfds, writefds, exceptfds; + NihError *err; + json_object *json; TEST_FUNCTION_FEATURE ("conf_source_reload", "with configuration file"); @@ -4500,12 +4501,76 @@ strcat (filename, "/bar.conf"); unlink (filename); + /* Re-enable inotify */ + assert0 (unsetenv ("INOTIFY_DISABLE")); + + TEST_FEATURE ("Invalid .conf file does not stop ConfFile being serialised"); + conf_init (); + TEST_LIST_EMPTY (conf_sources); + + f = fopen (filename, "w"); + /* Create an invalid job by adding an invalid stanza */ + fprintf (f, "invalid\n"); + fclose (f); + + source = conf_source_new (NULL, filename, CONF_FILE); + TEST_NE_P (source, NULL); + TEST_LIST_NOT_EMPTY (conf_sources); + TEST_HASH_EMPTY (source->files); + + file = conf_file_new (source, "/path/to/file"); + TEST_NE_P (file, NULL); + TEST_HASH_NOT_EMPTY (source->files); + + /* Initially, a ConfFile has no associated JobClass */ + TEST_EQ_P (file->job, NULL); + + /* Normally, this would create a JobClass and associate it with + * its parent ConfFile, but that doesn't happen when the on-disk + * job configuration file is invalid. + */ + ret = conf_source_reload (source); + + /* Although the on-disk file is invalid, there is no error here + * since it's already been handled by conf_reload_path() (which + * displays an error message with details of how the job + * configuration file is invalid). + */ + TEST_EQ (ret, 0); + + /* We know the job was invalid * by the fact that the ConfFile + * still has no associated JobClass. + */ + TEST_EQ (file->job, NULL); + + /* See if we can serialise the ConfFile without an associated + * JobClass. + */ + json = conf_file_serialise (file); + TEST_NE_P (json, NULL); + + /* Test there is no JobClass in the JSON */ + TEST_EQ_P (json_object_object_get (json, "job_class"), NULL); + + TEST_FEATURE ("ConfFile with no JobClass can be deserialised"); + + nih_free (source); + + source = conf_source_new (NULL, filename, CONF_FILE); + TEST_NE_P (source, NULL); + TEST_LIST_NOT_EMPTY (conf_sources); + TEST_HASH_EMPTY (source->files); + + file = conf_file_deserialise (source, json); + TEST_NE_P (file, NULL); + + nih_free (source); + + json_object_put (json); + + unlink (filename); rmdir (dirname); - nih_log_set_priority (NIH_LOG_MESSAGE); - - /* Re-enable inotify */ - assert0 (unsetenv ("INOTIFY_DISABLE")); }
-- upstart-devel mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/upstart-devel
