[systemd-devel] [PATCH 2/4] bus: driverd: don't remove twice a match from the list

[Lennart Poettering]

On Sat, 28.12.13 13:54, Marc-Antoine Perennou (Marc-Antoine at Perennou.com) wrote:

> match_free already does it
> ---
>  src/bus-driverd/bus-driverd.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/src/bus-driverd/bus-driverd.c b/src/bus-driverd/bus-driverd.c
> index 44172c4..b423420 100644
> --- a/src/bus-driverd/bus-driverd.c
> +++ b/src/bus-driverd/bus-driverd.c
> @@ -129,10 +129,8 @@ static int match_new(Client *c, struct bus_match_component *components, unsigned
>          first = hashmap_get(c->matches, m->match);
>          LIST_PREPEND(matches, first, m);
>          r = hashmap_replace(c->matches, m->match, first);
> -        if (r < 0) {
> -                LIST_REMOVE(matches, first, m);
> +        if (r < 0)
>                  goto fail;
> -        }

Hmm, did you run into an actual problem with this?

match_free() only undoes the registration if m->client is actually
set. Which it isn't in this case, since we actually set m->client after
the hashmap_replace() succeeded. Actually, we use whether m->client is
set or not as indication whether we have to remove the match from the
hashmap and the list that is appended to each entry.

To me it appears the currently code is quite correct, or am I missing
anything?

Lennart

-- 
Lennart Poettering, Red Hat