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

Jakub Hrozek jhrozek at redhat.com
Mon Aug 25 16:21:05 UTC 2014


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?

> 
> 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



More information about the sssd-devel mailing list