[systemd-devel] [PATCH 1/3] bus: add sd_bus_message_read_strv

[Lennart Poettering]

On Tue, 05.11.13 21:42, Marc-Antoine Perennou (Marc-Antoine at Perennou.com) wrote:

> It will be useful to have that in the public API

Applied! Thanks!

> 
> Signed-off-by: Marc-Antoine Perennou <Marc-Antoine at 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