On 05/06/2010 02:32 PM, Stephen Gallagher wrote:
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.
Sorry, accidentally attached the old patch. This should be the right one
now.
--
Stephen Gallagher
RHCE 804006346421761
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/