[PATCHS] fix handling of AUX classes in objectclass.c

[brendan powers]

On Tue, Dec 15, 2009 at 12:23 AM, Andrew Bartlett <abartlet at samba.org> wrote:
> On Mon, 2009-12-14 at 21:16 -0500, brendan powers wrote:
>> These patches fix the handling of AUX classes in objectclass.c. Before
>> this, adding an AUX class such as posixAccount to a user would fail
>> with LDB_ERR_OBJECT_CLASS_VIOLATION.
>
> Great.
>
>> Here is a description of the patches
>
> Can you please put these in the commit message for each patch?  We want
> to remember why these were done in the GIT history.

Sure

>> * 0001-return-NULL-in-strlower_talloc-if-src-is-NULL.patch, prevents
>> strlower_talloc from segfaulting if you pass it a NULL string.
>> * 0002-s4-dsdb-Add-a-check-to-prevent-acl_modify-from-debu.patch,
>> Check to see if there were any messages passed to acl_modify before
>> debugging the first one. I think I caused this by some malformed
>> ldiff. Unfortunately, I don't have the file that caused the problem.
>> * 0003-s4-dsdb-Move-get_last_structural-class-from-descrip.patch, Move
>> get_last_structural_class from descriptor.c to util.c so it can be
>> used by objectclass.c. Also, make get_last_structural_class ignore AUX
>> classes, as they are not structural classes.
>> * 0004-s4-dsdb-return-an-error-if-samAccountName-is-not-sp.patch,
>> makes sure samAccountName has been specified before adding a user.
>> This happened while I was trying to add a user with the posixAccount
>> objectclass. I forgot to specify the user objectClass, and samba
>> segfaulted. It now returns LDB_ERR_CONSTRAINT_VIOLATION.
>> * 0005-s4-dsdb-fix-handling-of-AUX-classes-in-objectclass_.patch, this
>> is the main patch in this set. It fixes the handling of AUX classes in
>> objectclass_sort. They were being sorted by building a class tree, and
>> adding the classes to the list in that order. However, AUX classes
>> usually don't fit into that tree, so LDB_ERR_OBJECT_CLASS_VIOLATION
>> was returned. I have changed the behavior to sort the classes by
>> subClass_order instead. Also this patch makes objectclass_do_add check
>> if the last structural class is a valid class to add, instead of the
>> last class returned by objectclass_sort.
>
> If it could be made to work, I would really prefer the qsort() variant
> of this patch.  Did you discover anything in the building of this patch
> to indicate why it didn't work?

The objectclass_sort function often adds important classes to the
list. One example is the domain class to the base DN during
provisioning. In doing so it changes the length of the array. So to
use the qsort function, I would need to allocate a new array, copy the
objects into it, sort it with qsort, and copy it back out to a linked
list. For lists usually less than 5 items, I'm not sure its worth it.
I can certainly do this though, if you want me to.

>> * 0006-s4-dsdb-Add-a-test-for-adding-deleting-and-append.patch, add a
>> test to make sure you can add the posixAccount class to an object.
>> This was the original reason for this set of patches.
>>
>> These patches now pass make test. So I thought it was time to send
>> them to the list. Hopefully I did things right this time:)
>
> Almost :-)
>
> Finally, was it deliberate having a different e-mail address in the
> patches to the mail?  (At least it's no longer root!).

That is the address I like to be contacted at, as I check it more
often. I'l change it to the address I use on this list, as it makes
more sense.

> Thanks,
>
> Andrew Bartlett
>
> --
> Andrew Bartlett                                http://samba.org/~abartlet/
> Authentication Developer, Samba Team           http://samba.org
> Samba Developer, Cisco Inc.
>
>