On Tue, 05.11.13 21:42, Marc-Antoine Perennou (marc-anto...@perennou.com) wrote:
> It will be useful to have that in the public API Applied! Thanks! > > Signed-off-by: Marc-Antoine Perennou <marc-anto...@perennou.com> We don't do S-o-b in systemd. Hence I dropped this before applying. > +int sd_bus_message_read_strv(sd_bus_message *m, char ***l) { > + char **strv = NULL; > + int r; > + > + assert(m); > + assert(l); For public APIs we need to be more forgiving thatn for our internal APIs. This means while it is OK to assert() on programming errors for internal code it is not OK to do this when other developers make use of our interfaces. Instead they should get a clean error code. The recommended way to do this is via the assert_return() macro. I made the necessary changes before commiting. > + > + r = bus_message_read_strv_extend(m, &strv); > + if (r < 0) > + return r; Hmm, so we should try to keep the End-Of-Container handling somewhat uniform. This means the read calls should return > 0 if they managed to read an item, 0 if we were at the end of an array (and only then), and negative on error. I fixed this accordingly here. Also _strv_extend() doesn't cleanup the list on failure (which is one of the reasons I didn't want this exposed. Fixed that too. Thanks! Lennart -- Lennart Poettering, Red Hat _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel