[systemd-devel] [PATCH] domain: fix a BUG_ON() when kdbus_domain_new() fails

[Kay Sievers]

On Mon, Jun 2, 2014 at 5:57 PM, Djalal Harouni <tixxdz at opendz.org> wrote:
> Currently just running: test/test-kdbus will trigger the BUG_ON()
> appended at the bottom.
>
> This is due to the test in check_domain_make() where we try to register
> the same domain twice line: 297, hence kdbus_domain_new() fails with
> -EEXIST at line domain.c:289
>
> Later on error path we clear the non-finalized domain:
> kdbus_domain_unref()
>  => __kdbus_domain_free()
>    => BUG_ON(!domain->disconnected)

Oh, hmm, I should probably run our own tests from time to time, not
just the full system as a test. :)

> After a closer look, it seems we will hit this BUG_ON() on every time
> kdbus_domain_new() fails. domain was not finalized so
> kdbus_domain_disconnect() is never called, and domain->disconnect can't
> be true.
>
> To fix this, I just set 'domain->disconnect = true' at the beginning
> which is perfectly true since that domain is not finalized hence not
> connected, and before we return success set it again to 'false' in other
> words: connected.
>
> I just took this path since it seems logic, and having a single exit
> node "kdbus_domain_unref()" on success/errors wich passes all these
> BUG_ON() makes the code robust.
>
> In other places: bus, endpoints we do not follow this and we duplicate
> the unref() logic, for endpoints it would be easy to convert! I'll
> probably follow with patches for that.

It was that way earlier, it will probably not work out for the others.
We needed to open-code it because the objects were not fully
initialized and the rollback was not possible without doing weird
things.

I actually think adding the needed 3 free() calls here, like for the
other objects, is shorter, needs no explanation and is easier to
follow.

Kay