On 05/06/2010 02:11 PM, Dmitri Pal wrote:
Stephen Gallagher wrote:
> ctx->gsh is sometimes NULL here, so dereferencing it without checking
> is bad.
> ------------------------------------------------------------------------
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)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?
It is now :)
2) What happens if the ctx->gsh is actually null? Is it safe to
fall
through. May be a comment would help.
Comment added.
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.
The third case was wrong. Good catch! I have now fixed this.
New patch attached. Thanks for the review.
--
Stephen Gallagher
RHCE 804006346421761
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/