[SSSD] [PATCH] SDAP: rem warning - sizelimit exceeded in POSIX check

Michal Židek mzidek at redhat.com
Fri Oct 9 18:02:08 UTC 2015


On 10/09/2015 02:05 PM, Lukas Slebodnik wrote:
> On (09/10/15 13:56), Jakub Hrozek wrote:
>> On Tue, Oct 06, 2015 at 01:25:33PM +0200, Pavel Reichl wrote:
>>>
>>>
>>> On 10/06/2015 11:21 AM, Jakub Hrozek wrote:
>>>>
>>>> I personally don't care about unsigned vs unsigned int, but see my
>>>> comment about the request inline..
>>>>
>>>>>  From 2281410185205ab3fc483f4c45b1b1378b62c331 Mon Sep 17 00:00:00 2001
>>>>> From: Pavel Reichl <preichl at redhat.com>
>>>>> Date: Fri, 25 Sep 2015 07:05:30 -0400
>>>>> Subject: [PATCH] SDAP: rem warning - sizelimit exceeded in POSIX check
>>>>>
>>>>> Add new parameter 'flags' to sdap_get_generic_ext_send_ext() which can
>>>>> be set to suppress warning about 'sizelimit exceeded'.
>>>>>
>>>>> Resolves:
>>>>> https://fedorahosted.org/sssd/ticket/2804
>>>>> ---
>>>>>   src/providers/ldap/sdap_async.c | 78 +++++++++++++++++++++++++++++------------
>>>>>   1 file changed, 56 insertions(+), 22 deletions(-)
>>>>>
>>>>> diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
>>>>> index 97c9ea5df61a6516ca74bb73edc9a116b1266c71..0044ea9c319af08a87fa59e89559851ad6e8abc5 100644
>>>>> --- a/src/providers/ldap/sdap_async.c
>>>>> +++ b/src/providers/ldap/sdap_async.c
>>>>> @@ -1164,6 +1164,8 @@ struct sdap_get_generic_ext_state {
>>>>>       void *cb_data;
>>>>>
>>>>>       bool allow_paging;
>>>>> +
>>>>> +    unsigned int flags;
>>>>>   };
>>>>>
>>>>>   static errno_t sdap_get_generic_ext_step(struct tevent_req *req);
>>>>> @@ -1172,23 +1174,26 @@ static void sdap_get_generic_op_finished(struct sdap_op *op,
>>>>>                                            struct sdap_msg *reply,
>>>>>                                            int error, void *pvt);
>>>>>
>>>>> +#define WARN_SIZELIMIT 1
>>>>> +
>>>>>   static struct tevent_req *
>>>>> -sdap_get_generic_ext_send(TALLOC_CTX *memctx,
>>>>> -                          struct tevent_context *ev,
>>>>> -                          struct sdap_options *opts,
>>>>> -                          struct sdap_handle *sh,
>>>>> -                          const char *search_base,
>>>>> -                          int scope,
>>>>> -                          const char *filter,
>>>>> -                          const char **attrs,
>>>>> -                          int attrsonly,
>>>>> -                          LDAPControl **serverctrls,
>>>>> -                          LDAPControl **clientctrls,
>>>>> -                          int sizelimit,
>>>>> -                          int timeout,
>>>>> -                          bool allow_paging,
>>>>> -                          sdap_parse_cb parse_cb,
>>>>> -                          void *cb_data)
>>>>> +sdap_get_generic_ext_send_ext(TALLOC_CTX *memctx,
>>>>> +                              struct tevent_context *ev,
>>>>> +                              struct sdap_options *opts,
>>>>> +                              struct sdap_handle *sh,
>>>>> +                              const char *search_base,
>>>>> +                              int scope,
>>>>> +                              const char *filter,
>>>>> +                              const char **attrs,
>>>>> +                              int attrsonly,
>>>>> +                              LDAPControl **serverctrls,
>>>>> +                              LDAPControl **clientctrls,
>>>>> +                              int sizelimit,
>>>>> +                              int timeout,
>>>>> +                              bool allow_paging,
>>>>> +                              sdap_parse_cb parse_cb,
>>>>> +                              void *cb_data,
>>>>> +                              unsigned int flags)
>>>>
>>>> I don't like us adding another request below sdap_get_generic_ext_send.
>>>
>>> OK, I understand that.
>>>
>>>> IIRC the idea was that sdap_get_generic_ext_send is really raw and
>>>> unless you need the raw access you should use sdap_get_generic_send
>>>> (without _ext).
>>>>
>>>> So here I would prefer to return ERR_SIZELIMIT_EXCEEDED from the _ext request
>>>> and let callers deal with the error. The posix check caller would ignore
>>>> it, the others might display an error and carry on.
>>> This sound to me like code duplication. I would have to add to several calls same check and print same message.
>>
>> This is totally untested version of what I meant:
>>     https://fedorapeople.org/cgit/jhrozek/public_git/sssd.git/commit/?h=misc
>>
> It looks more elegant then adding flags to function.
> So +1
>
> But there is still a change that it does not work because patches are totally
> untested :-)
>
> LS

I agree with Pavel that ignoring the debug message is non standard
thing that we want to do only during the posix check and it does not
add much readbility/ease of use when the caller needs to stalk for
the special error code (only in one case it will be ignored).
I also agree with Jakub that we do not want to expand already big
number of arguments.

Please see the attached patch. It is less invasive and only changes
parameters for the posix check (for now the one and only special
case when we want to ignore the debug message).

Michal

PS: The patch is also totally untested and serves for illustration,
but it is simple enough to accidentally work :)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-sdap-Ignore-sizelimit-during-posix-check.patch
Type: text/x-patch
Size: 2539 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151009/68ba830b/attachment.bin>


More information about the sssd-devel mailing list