[SSSD] [PATCHES] Add support for paged LDAP results

Jakub Hrozek jhrozek at redhat.com
Tue Apr 26 15:37:34 UTC 2011


On 04/25/2011 10:52 PM, Simo Sorce wrote:
> On Mon, 2011-04-25 at 16:41 -0400, Stephen Gallagher wrote:
>> On Mon, 2011-04-25 at 16:21 -0400, Stephen Gallagher wrote:
>>> On Mon, 2011-04-25 at 15:33 -0400, Stephen Gallagher wrote:
>>>> On Mon, 2011-04-25 at 14:26 -0400, Stephen Gallagher wrote:
>>>>> Patch 0001: Added a debug message to see which record type we're
>>>>> processing on each loop through sdap_process_message(). This is purely
>>>>> informational.
>>>>>
>>>>> Patch 0002: Add support for paged LDAP results.
>>>>> I changed the internals of sdap_get_generic_send() somewhat here and
>>>>> added a new sdap_get_generic_internal() routine that can be used to
>>>>> handle multiple calls to LDAP for a single request. We should be able to
>>>>> use this in the future to handle Active Directory's non-standard
>>>>> attribute-level paging as well (though that's not addressed in this
>>>>> patch).
>>>>>
>>>>> Patch 0003: Add ldap_page_size configuration option
>>>>> I made this a separate patch for simplicity of review. I set 1000
>>>>> records as the default, as this seemed to be the most-compatible value
>>>>> among 389, OpenLDAP and ActiveDirectory as best I could determine.
>>>>
>>>>
>>>> Simo made some comments on IRC that I have incorporated.
>>>>
>>>> First, I renamed sdap_get_generic_send_internal() to
>>>> sdap_get_generic_step() and reordered it according to the tevent_req
>>>> style. Second, I added talloc_zfree(state->op) to the beginning of
>>>> sdap_get_generic_step() to ensure that we aren't carrying around extra
>>>> sdap_op memory for subsequent pages.
>>>
>>> Simo had a few remaining comments. To follow tevent_req style, I've
>>> moved forward declarations to be above the sdap_get_generic_send().
>>>
>>> Also, there was a memory issue (I was accessing memory that had been
>>> leaked). The cookie had gone out of scope, but I hadn't freed the memory
>>> (which is why I saw no issues in my tests).
>>>
>>> I now make an appropriate copy of the cookie berval and free the
>>> original memory.
>>
>> One more change: the cookie is now copied with talloc_memdup() instead
>> of talloc_strdup(), since it may not be a proper string (I'm looking at
>> YOU Active Directory).
> 
> Ack,
> although it would be nice to have someone else test it before actually
> pushing to master.
> 

I tested it against our corporate LDAP server plus my devel IPA server.
Seems to work just fine.

I have two comments, though - ldap_create_page_control() and
ldap_parse_pageresponse_control() don't seem to exist on RHEL5. We
should probably #ifdef this functionality out (or implement it for old
libldap releases).

Also the "paging_criticality" variable is char, but the iscritical
parameter of ldap_parse_pageresponse_control() is declared as int.

The rest looks good to me.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 261 bytes
Desc: OpenPGP digital signature
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20110426/b67dde18/attachment.sig>


More information about the sssd-devel mailing list