[PATCH] lib/ldb minor fixes

[Kamen Mazdrashki]

On Fri, Nov 14, 2014 at 10:56 PM, Kamen Mazdrashki <kamenim at samba.org>
wrote:

> Hi Andrew,
>
> On Fri, Nov 14, 2014 at 8:33 PM, Andrew Bartlett <abartlet at samba.org>
> wrote:
>
>> On Fri, 2014-11-14 at 04:16 +0100, Kamen Mazdrashki wrote:
>> > Hi Andrew,
>> >
>> > On Fri, Nov 14, 2014 at 2:35 AM, Kamen Mazdrashki <kamenim at samba.org>
>> wrote:
>> >
>> > >
>> > >
>> > > On Thu, Nov 13, 2014 at 5:43 AM, Andrew Bartlett <abartlet at samba.org>
>> > > wrote:
>> > >
>> > >>
>> > >> I still don't like the check for a == NULL at:
>> > >>
>> > >>
>> https://github.com/kamenim/samba/commit/2966a44cd900f597aed232c963693ab85bd33f4b#diff-d7e2445ee96f1098ad238e62cf47cb92R323
>> > >>
>> > >> The reason I don't like it is that this function is designed not to
>> > >> fail, and if we start checking here, for a condition that can't
>> happen,
>> > >> then we will start checking everywhere, as folks work to make it
>> > >> consistent.
>> > >>
>> > >
>> > >
>> > > Well, I have changed it to fall explicitely in case 'name == null'
>> :). See:
>> > >
>> > >
>> https://github.com/kamenim/samba/commit/194658167d65f0f96e7150b1f5e5652175830999#diff-a410d07b1af338be3b0b3d1c3d62622aR131
>> > >
>> > > I didn't know "no fail" is the intended design as half of the calls to
>> > > this function
>> > > actually check the return value.
>> > > I will change then
>> > >
>> > >
>> https://github.com/kamenim/samba/commit/194658167d65f0f96e7150b1f5e5652175830999#diff-a410d07b1af338be3b0b3d1c3d62622aR131
>> > > to always succeed and remove the check that bothers you. It is not
>> needed
>> > > indeed.
>> > >
>> >
>> > I have updated the branch (PR): https://github.com/kamenim/samba/pull/4
>> >
>> > Reworked commits are:
>> >
>> https://github.com/kamenim/samba/commit/dd677c73377eae6d3f317c97e7adccfcf85cbaa8
>> >
>> https://github.com/kamenim/samba/commit/888a764d3045bca86a8143fe3f0d2135628fb149
>>
>> Thanks.  I've been trying to figure out why I didn't quite feel right
>> about this patch.  Belts and braces are an OK strategy, and can avoid
>> crashes after an unchecked allocation failure elsewhere, but shouldn't
>> we be checking for name == NULL in the python LDB bindings?
>>
>> Yes, we should do that also. My plan was to fix the root implementation -
> more or less
> ment as a "one fix to rule them all" and make sure this won't left hidden
> anymore.
> At the same time, my primary goal wasn't to fix param checks everywhere
> (i.e. fix the root and continue with what I was doing already)
>
> I can take a look for other uses of course too
>
> Look taken.
I can't find out how we could avoid name == NULL in python bindings.
PyLdbMessageElement by design allows optional parameters in constructor
(name = NULL)
Honestly said, I don't see what an empty PyLdbMessageElement could be used
for
(object interface is close to readonly as far as I can see)

The only optoin I could see here is to change PyLdbMessageElement
constructor to require
name and elements to be non-optional.
This may affect lots of code though, because making 'elements' and 'name'
non-optional
and leaving 'flags' optional will require change in interface (and tests
and so on)

<<including Jelmer as author for this code>>

Cheers,
Kamen