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

2013-12-28 Thread Lennart Poettering
On Sat, 28.12.13 13:54, Marc-Antoine Perennou (marc-anto...@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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2013-12-27 Thread Marc-Antoine Perennou
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;
-}
 
 m-client = c;
 c-n_matches++;
-- 
1.8.5.2

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel