Stephen Gallagher wrote: > ctx->gsh is sometimes NULL here, so dereferencing it without checking > is bad. > ------------------------------------------------------------------------ > > _______________________________________________ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://fedorahosted.org/mailman/listinfo/sssd-devel Through I do not have a problem with this specific patch I do have several concerns about the underlaying code.
1) The code seems to be duplicated multiple times. Why common code is not segmented into a common function? 2) What happens if the ctx->gsh is actually null? Is it safe to fall through. May be a comment would help. 3) The code around sdap_account_info_restart(breq); is not consistent. In first two cases we return in case of success. What do we do in case of failure? In the last function we on the contrary do not check the error and return right away regardless of the outcome of the sdap_account_info_restart(breq). This does not look right. -- Thank you, Dmitri Pal Engineering Manager IPA project, Red Hat Inc. ------------------------------- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel