On Tue, Jun 03, 2014 at 12:55:54PM +0200, Kay Sievers wrote: > On Mon, Jun 2, 2014 at 5:57 PM, Djalal Harouni <[email protected]> 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. Ok. > 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. Ok, I'll do that, and it seems we do not clean the chrdev and idr too, will send a patch. Thanks > Kay -- Djalal Harouni http://opendz.org _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
