[PATCHES] Cleanup dead records in messages.tdb

Christof Schmitt cs at samba.org
Thu Apr 3 15:26:29 MDT 2014


On Wed, Apr 02, 2014 at 09:38:58PM -0700, Christof Schmitt wrote:
> On Wed, Apr 02, 2014 at 06:06:47PM -0700, Jeremy Allison wrote:
> > On Wed, Apr 02, 2014 at 05:21:51PM -0700, Christof Schmitt wrote:
> > > minor question would be why you have messaging_tdb_cleanup return a
> > > status, only to ignore it in the caller.
> > 
> > We ignore it *now*, as there's not much
> > we can really do with it (maybe DEBUG
> > message ?). But it is good pactice to
> > return NTSTATUS error codes whenever
> > possible, so I always try and do so.
> 
> I would have made the function return nothing, and only add the returned
> status when it is actually needed. Anyway, this is just a minor point.
> 
> > 
> > > Overall it looks good:
> > > Reviewed-by: Christof Schmitt <cs at samba.org>
> > 
> > Thanks ! I'll push tomorrow ! Do you want to
> > log a bug so we can get it back-ported to
> > 4.0.next and 4.1.next ?
> 
> https://bugzilla.samba.org/show_bug.cgi?id=10534

Actually, there is an issue with the locking here: tdb_delete fails,
because it tries to get a lock for the delete, but the chain is already
locked. Either we have to live with the race here, or add a call to tdb
that allows deletion without acquiring the lock.

Christof


More information about the samba-technical mailing list