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(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel