[PATCH] lib/ldb minor fixes

[Kamen Mazdrashki]

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

Cheers,
Kamen