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

[brendan powers]

Here are the updated patches with a new email address and more verbose
commit messages.

On Wed, Dec 16, 2009 at 3:05 PM, Andrew Bartlett <abartlet at samba.org> wrote:
> On Tue, 2009-12-15 at 10:14 -0500, brendan powers wrote:
>> 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.
>
> Ahh, I didn't realise it did that too.  It's been a while since I wrote
> this horror - the odd thing is, while believe you, reading the code I
> still can't see how it does it!
>
>> >> * 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.
>
> Either is fine, I just wanted to check.
>
> Andrew Bartlett
>
> --
> Andrew Bartlett                                http://samba.org/~abartlet/
> Authentication Developer, Samba Team           http://samba.org
> Samba Developer, Cisco Inc.
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-return-NULL-in-strlower_talloc-if-src-is-NULL.patch
Type: text/x-diff
Size: 982 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091216/6de69b4f/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-s4-dsdb-Add-a-check-to-prevent-acl_modify-from-debu.patch
Type: text/x-diff
Size: 1206 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091216/6de69b4f/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-s4-dsdb-Move-get_last_structural-class-from-descrip.patch
Type: text/x-diff
Size: 3515 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091216/6de69b4f/attachment-0002.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-s4-dsdb-return-an-error-if-samAccountName-is-not-sp.patch
Type: text/x-diff
Size: 1283 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091216/6de69b4f/attachment-0003.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-s4-dsdb-fix-handling-of-AUX-classes-in-objectclass_.patch
Type: text/x-diff
Size: 15986 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091216/6de69b4f/attachment-0004.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-s4-dsdb-Add-a-test-for-adding-deleting-and-append.patch
Type: text/x-diff
Size: 2146 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091216/6de69b4f/attachment-0005.patch>