[PATCH] lib/tdb: locking cleanup

Andrew Bartlett abartlet at samba.org
Fri Aug 28 00:23:16 MDT 2009


On Fri, 2009-08-28 at 14:43 +0930, Rusty Russell wrote:
> On Fri, 28 Aug 2009 01:25:47 pm tridge at samba.org wrote:
> > Hi Rusty,
> > 
> >  > -int tdb_brlock(struct tdb_context *tdb, tdb_off_t offset, 
> >  > -	       int rw_type, int lck_type, int probe, size_t len)
> >  > +int tdb_brlock(struct tdb_context *tdb,
> >  > +	       int rw_type, tdb_off_t offset, size_t len,
> >  > +	       enum tdb_lock_flags flags)
> >  >  {
> >  > -	struct flock fl;
> >  >  	int ret;
> >  >  
> >  >  	if (tdb->flags & TDB_NOLOCK) {
> >  >  		return 0;
> >  >  	}
> >  >  
> >  > +	if (flags & TDB_LOCK_MARK_ONLY) {
> >  > +		return 0;
> >  > +	}
> > 
> > Using an enum as a bitmask seems a bit strange. It's valid C, but
> > anyone reading it will think that the enum takes on the set of values
> > listed in the enum declaration, and may do something like a switch
> > statement with the various cases. Why use an enum when you mean it to
> > be masked like this?
> 
> Habit, probably.  I was originally going to ditch "mark" altogether, but
> that seems a while off.
> 
> I'll change to #defines.
> 
> > btw, the only user I know of the 'mark' locks is ctdb. Have you or
> > Ronnie tested this patch with ctdb?
> 
> No.  Which brings us to a discussion of unit tests.  I have some unit tests
> for tdb (in ccan), and I'd really like to have them in samba, esp. for the lib
> dir.  Higher-level testing is great, but unit tests are better for isolating
> bugs and testing nasty corners.
> 
> The ccan unit tests sit in a test/ operate as so:
> 1) C files starting with "api" are compiled and linked against the library,
>    then executed.  These should work on all future versions of the lib.
> 2) C files starting with "run" are compiled and run (usually, they explicitly
>    #include the C files from .. so they can test static funcs and override
>    internal stuff.  Works really well in practice).
> 3) C files starting with "compile_ok" or "compile_fail" are simply compiled
>    and linked.  Not sure we need these.
> 4) All other C files are linked into everything, for helper routines.
> 
> I use libtap, which creates simple one-line-per-test output (the TAP output
> format is standardized see testanything.org)

Any chance you can use the subunit format already used by other parts of
Samba?  (Samba4's 'make test' in particular). 

We have formatters and test parsers built around this format already.

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Cisco Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20090828/183a5a39/attachment.pgp>


More information about the samba-technical mailing list