--- init/event_operator.h 2013-02-27 11:46:04 +0000
+++ init/event_operator.h 2013-05-30 17:07:37 +0000
[...]
+event_operator_deserialise_all (void *parent, json_object *json)
+ __attribute__ ((warn_unused_result));
+
+NIH_END_EXTERN
+
char *event_operator_collapse (EventOperator *condition)
- __attribute__ ((warn_unused_result));
-
-NIH_END_EXTERN
+ __attribute__ ((warn_unused_result, unused));
This moves the event_operator_collapse() definition outside the extern "C" {}
block, which seems like a mistake?
--- init/job_class.c 2013-05-15 13:21:54 +0000
+++ init/job_class.c 2013-05-30 17:07:37 +0000
@@ -1910,17 +2015,17 @@
json_class = job_class_serialise (class);
- /* No object returned means the class doesn't need to be
- * serialised. Even if this is a real failure, it's always
- * better to serialise as much of the state as possible.
- */
if (! json_class)
- continue;
+ goto error;
json_object_array_add (json, json_class);
I think the reasoning in that comment still holds (that it's better to
serialize as much of the state as possible). But this is consistent with the
behavior in the rest of the code, so I suppose that shouldn't be a blocker.
--- init/state.c 2013-02-28 16:40:36 +0000
+++ init/state.c 2013-05-30 17:07:37 +0000
[...]
+ /* Again, we cannot error here since older JSON state data did
+ * not encode ConfSource or ConfFile objects.
+ */
+ if (conf_source_deserialise_all (json) < 0)
+ nih_warn ("%s", _("No ConfSources present in state data"));
Wouldn't it make sense to have a check on the format version, and make this an
error for new formats vs. a warning for the old format?
So given that there are version checks now in the deserializer, and there is
another change to the serialization format landing upstream at the same time:
how can we backport this change to raring in an SRU? Should we consider the
format with apparmor_switch to be version 4, and treat the confsource
serialization without apparmor_switch to be version 3, then include tests for
each (i.e., change
test_upstart_pre_security_upgrade() to use v3)?
Finally, it appears that there is no test json for the *current* serialization
format. This should definitely be included as its own test, and added to the
tree now - not be left until the next time someone changes the serialization
format (or accidentally changes the deserializer!).
Other than that, this looks very good. If you can fix up the above issues,
I'll approve this.
--
https://code.launchpad.net/~jamesodhunt/upstart/serialise-remaining-objects/+merge/163151
Your team Upstart Reviewers is subscribed to branch lp:upstart.
--
upstart-devel mailing list
[email protected]
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/upstart-devel
