[SSSD] [PATCH] LDAP: Don't add a user member twice when adding a primary group

Pavel Reichl preichl at redhat.com
Mon Aug 25 18:18:15 UTC 2014


On 08/25/2014 06:21 PM, Jakub Hrozek wrote:
> On Mon, Aug 25, 2014 at 05:23:44PM +0200, Pavel Reichl wrote:
>> Hello,
>>
>> I tested the patch and it fixes the problem described in the ticket.
>>
>> I personally dislike the break and mainly the continue check - I believe
>> that people reading the code would benefit from encapsulating the inner loop
>> into function (return value would indicate what is happening and function
>> name is the best comment anyway) and same would go for the outer loop IMO.
> That's a fair comment. Would you prefer to split the for loop into one
> function or two?
two, but I can live with one :-)
>
>> Still I am aware that this is a taste of personal preference and I'm willing
>> to ACK the patch even without the changes.
>>
>> Thanks!
>>
>> On 08/17/2014 08:47 PM, Jakub Hrozek wrote:
>>> +        for (ii = 0; ii < el->num_values; ii++) {
>>> +            if (strcmp(userdns[i], (const char *) el->values[ii].data) == 0) {
>>> +                DEBUG(SSSDBG_TRACE_INTERNAL,
>>> +                      "Member %s already included, skipping\n", userdns[i]);
>>> +                break;
>>> +            }
>>> +        }
>>> +
>>> +        if (ii < el->num_values) {
>>> +            continue;
>>> +        }
>> _______________________________________________
>> sssd-devel mailing list
>> sssd-devel at lists.fedorahosted.org
>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> _______________________________________________
> 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