LDB: Patch for fixing counter variables

Andrew Bartlett abartlet at samba.org
Mon Nov 9 17:57:33 MST 2009


On Mon, 2009-11-09 at 16:28 +0100, Matthias Dieter Wallnöfer wrote:
> Andrew,
> 
> I dropped the wrong part of the patch though your and Jelmer's 
> suggestion. The other changes should fully compliy - I would like to 
> remember: "make test" passes as without them - so nothing (additional) 
> should have been broken.
> 
> I hope that you aren't that picky on those changes since then we never 
> get to an end. For the future I plan also to provide some patches on 
> other similar s4 code. Of course if you notice an obvious mistake (like 
> this with python) please inform me. But otherwise going through each 
> part I don't see very efficient.

It isn't very efficient, but just like the const changes it is easy to
slip in other changes and subtle bugs in a big 'cleanup' patch. 

That's why we need to be careful doing this, and why splitting things up
helps, because it allows me or others to say '1 is ok, still thinking
about 2'.  The more you split it up, the easier it is for me.

Also, for me the whole question of 'is there any point' is still very
much open. 

That is, the boundary condition needs to be *way* below 2^31 (can we
really allocate that many elements?), and raising it to 2^32 just seems
to change the rollover point.  
BTW, See: http://xkcd.com/571/

Most of the one I've looked at seem OK, but because it's so large I've
not been able to look at them all.  However some things look like they
could be improved, such as the change to ldb_msg_remove_element(), which
looks like it should be a ptrdiff_t, and check for both above and below
the range.  

And this change:
 char *ldb_binary_encode(void *mem_ctx, struct ldb_val val)
 {
-	int i;
 	char *ret;
-	int len = val.length;
+	unsigned int i, len = val.length;
 	unsigned char *buf = val.data;

Would have been better as just a search/replace - the construction 'i,
len = val.length' isn't as nice.

I'm sorry this is so tedious, and to be so picky.  Like the const
changes, it will take time to check all these (and 'make test' generally
isn't sufficient on these kind of changes). 

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/20091110/e4071031/attachment.pgp>


More information about the samba-technical mailing list