[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