[PATCH] lib/ldb minor fixes

[Andrew Bartlett]

On Wed, 2014-11-12 at 01:28 +0100, Kamen Mazdrashki wrote:
> Hi Andrew,
> 
> I have updated the patches according our discussion. See inline.
> Also, there is one more commit that allows for adding MessageElements
> from one Message object to another - at the moment such attempts lead
> to crashes due to "Bad talloc magic value" error
> 
> As usual, attached is the whole patchset.
> A pull request for convenient review is here:
>   https://github.com/kamenim/samba/pull/4
> 
> On Tue, Nov 11, 2014 at 10:29 AM, Kamen Mazdrashki <kamenim at samba.org>
> wrote:
> 
> > Hi Andrew,
> >
> > On Tue, Nov 11, 2014 at 9:14 AM, Andrew Bartlett <abartlet at samba.org>
> > wrote:
> >
> >> On Tue, 2014-11-11 at 05:29 +0100, Kamen Mazdrashki wrote:
> >> > I have update the last patch to this:
> >> >
> >> https://github.com/kamenim/samba/commit/be1808de8bc5232dd9b827f93ab0d3151653ff88
> >> > Also attached the whole set to this mail.
> >>
> >>
> >> I still want it to instead check for
> >>
> >> if (msg->elements[i].name == NULL)
> >>
> >
> > I have  considered this also.
> > I have opted out for this check to be done
> > in ldb_schema_attribute_by_name_internal()
> > though, as every call for finding attribute by name end up being handled
> > by this function.
> > Hence the sanity check handles much more code paths.
> > I see the benefit of such check here though, so we can get a much better
> > error message
> >
> >
> Please take a look at updated patch here:
> 
> https://github.com/kamenim/samba/commit/2966a44cd900f597aed232c963693ab85bd33f4b
> Let me know if this is what you had in mind too.

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. 

The check for name == NULL seems reasonable, particularly if it can get
that way from the python.  

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba