LDB: Patch for fixing counter variables

[Matthias Dieter Wallnöfer]

Thanks Jelmer, for your clear explaination. I will drop the python part. To be honest I wasn't very about it.

Matthias

--- Jelmer Vernooij <jelmer at samba.org> schrieb am Sa, 7.11.2009:

Von: Jelmer Vernooij <jelmer at samba.org>
Betreff: Re: LDB: Patch for fixing counter variables
An: mdw at samba.org
CC: "Andrew Bartlett" <abartlet at samba.org>, "Samba Technical Mailinglist" <samba-technical at lists.samba.org>, idra at samba.org
Datum: Samstag, 7. November 2009, 11:02

Hi matthias,

On Sat, 2009-11-07 at 09:24 +0000, Matthias Dieter Wallnöfer wrote:
> I don't have much time to chat now, so I submit you a new version of the patch: divided in C and python bindings.
The Python binding changes are pointless. PyList_SetItem() takes an
integer (not an unsigned integer), so changing this doesn't fix
anything. If we really suspect we're going to run into ldb results or
messages with more than billions of elements, we should just raise an
exception if they don't fit in a Python list.

Cheers,

Jelmer

> --- Andrew Bartlett <abartlet at samba.org> schrieb am Sa, 7.11.2009:
> 
> Von: Andrew Bartlett <abartlet at samba.org>
> Betreff: Re: LDB: Patch for fixing counter variables
> An: mdw at samba.org
> CC: idra at samba.org, "Samba Technical Mailinglist" <samba-technical at lists.samba.org>
> Datum: Samstag, 7. November 2009, 1:37
> 
> On Fri, 2009-11-06 at 21:19 +0000, Matthias Dieter Wallnöfer wrote:
> > Most of our LDB objects are stored in structures containing a dynamic
> > array and a counter. The counter variable is typically an "unsigned
> > integer". Until here it's all okay. But with the major part of the
> > counter variables (i, j...) it's not: since they're "signed". Well,
> > this doesn't give problems till we don't break the ~ 2,1 * 10^9
> > barrier - but the API is fine to go beyond: with the possible result
> > of infinite loops (since we are never able to reach the maximum).
> > Now you can argue: "this does happen very rarely" - where I'm also
> > with you. But that doesn't mean that those counters aren't buggy.
> > Since the LDB is a public library for  LDAP-like directories I would
> > be very pleased to see this fixed.
> > To achieve this I prepared a patch which changes "signed" types to
> > "unsigned" ones which fix the problem. It passes "make test" and
> > doesn't seem to give problems. The only thing I would like to ask you
> > is to review it, so I could push it soon.
> 
> Matthias, 
> 
> As you may have come to expect, my first request is that you split the
> patch up.  There are parts of this that look safe, and many parts that
> need to looked at more carefully.
> 
> The changes the the Python look most dubious, as you are changing the
> type of something that is then interpreted as an integer in the python.
> (so please remove all the Python changes). 
> 
> You also have typo fixes and changes to use LDB_SUCCESS mixed in the
> same patch.  
> 
> Once you do that, I'll take another look, and we can see if some of
> these can be easily merged.   Perhaps also produce the patches with more
> context?  It's hard to tell from the changed declaration what the effect
> would be (otherwise I'll have to do the review in gitk). 
> 
> Thanks,
> 
> Andrew Bartlett
> 
> -- 
> Andrew Bartlett                                http://samba.org/~abartlet/
> Authentication Developer, Samba Team           http://samba.org
> Samba Developer, Cisco Inc.
> 
> 
> 
> 
>       


-- 
Jelmer Vernooij <jelmer at samba.org> - http://samba.org/~jelmer/
Jabber: jelmer at jabber.fsfe.org