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

Pavel Reichl preichl at redhat.com
Fri Sep 25 12:31:49 UTC 2015



On 09/25/2015 01:59 PM, Lukas Slebodnik wrote:
> On (25/09/15 13:30), Pavel Reichl wrote:
>> Hello,
>>
>> please see simple patch attached.
>>
>> Thanks!
>
>>From 5717c6effcb0ac0cd16b4863adba088c9b1f0b90 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() which can be
>> set to suppress warning about 'sizelimit exceeded'.
>>
>> Resolves:
>> https://fedorahosted.org/sssd/ticket/2804
>> ---
>> src/providers/ldap/sdap_async.c | 34 +++++++++++++++++++++++++---------
>> 1 file changed, 25 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
>> index 97c9ea5df61a6516ca74bb73edc9a116b1266c71..1626d0dada1800aaf69681de13632b007866e779 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;
>                 ^^^
>              The int is redundant here
>              "unsigned int" and "unsigned" is the same.

Indeed it's the same. But we use this form quite common. Do you want me to file a ticket and assign it to you?

git grep 'unsigned int' |  wc -l
206


>
>> };
>>
>> static errno_t sdap_get_generic_ext_step(struct tevent_req *req);
>> @@ -1172,6 +1174,14 @@ static void sdap_get_generic_op_finished(struct sdap_op *op,
>>                                           struct sdap_msg *reply,
>>                                           int error, void *pvt);
>>
>> +#define NO_FLAGS 0
>> +#define DONT_WARN_SIZELIMIT_EXCEEDED 1
>> +
>> +static bool is_flag_set(unsigned int flags, unsigned int flag)
>> +{
>> +    return (flags & flag) != 0;
>> +}
>> +
>> static struct tevent_req *
>> sdap_get_generic_ext_send(TALLOC_CTX *memctx,
>>                            struct tevent_context *ev,
>> @@ -1188,7 +1198,8 @@ sdap_get_generic_ext_send(TALLOC_CTX *memctx,
>>                            int timeout,
>>                            bool allow_paging,
>>                            sdap_parse_cb parse_cb,
>> -                          void *cb_data)
>> +                          void *cb_data,
>> +                          unsigned int flags)
>> {
>>      errno_t ret;
>>      struct sdap_get_generic_ext_state *state;
>> @@ -1215,6 +1226,7 @@ sdap_get_generic_ext_send(TALLOC_CTX *memctx,
>>      state->parse_cb = parse_cb;
>>      state->cb_data = cb_data;
>>      state->clientctrls = clientctrls;
>> +    state->flags = flags;
>>
>>      if (state->sh == NULL || state->sh->ldap == NULL) {
>>          DEBUG(SSSDBG_CRIT_FAILURE,
>> @@ -1500,8 +1512,11 @@ static void sdap_get_generic_op_finished(struct sdap_op *op,
>>
>>          if (result == LDAP_SIZELIMIT_EXCEEDED) {
>>              /* Try to return what we've got */
>> -            DEBUG(SSSDBG_MINOR_FAILURE,
>> -                  "LDAP sizelimit was exceeded, returning incomplete data\n");
>> +            if (!is_flag_set(state->flags, DONT_WARN_SIZELIMIT_EXCEEDED)) {
>                     The function is used just once and it does not significantly
>                     improve readability of code; But it might be caused by too
As negation is needed I believe it improves readability enough to advocate it's existence.

>                     long name of flag and many brackets + negation.
>                if (state->flags & WARN_SIZELIMIT) might be easier to read.

I don't like WARN_SIZELIMIT because setting any flag should not be required by default and this flag would imply otherwise.

>
> The simillar patter is used also on other places.
> src/sss_client/pam_sss.c:109:    if (err & PAM_DATA_REPLACE) {
> src/sss_client/pam_sss.c:1537:    if (flags & FLAGS_USE_FIRST_PASS) {
> src/sss_client/pam_sss.c:1546:        if (flags & FLAGS_USE_2FA
> src/sss_client/pam_sss.c:1561:        if (flags & FLAGS_FORWARD_PASS) {
> src/sss_client/pam_sss.c:1592:    if (pam_flags & PAM_PRELIM_CHECK) {
> ...
> and many other places
>
> sh$ git grep -n if | grep " & " | wc -l
> 177
>
> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>


More information about the sssd-devel mailing list