[SSSD] Dead assignment cleanup

Jan Zeleny jzeleny at redhat.com
Fri Sep 3 20:31:32 UTC 2010


Stephen Gallagher <sgallagh at redhat.com> wrote:
> On 09/03/2010 08:11 AM, Jan Zelený wrote:
> > 0001-Dead-assignments-cleanup-in-providers-code.patch:
> > Here the biggest change is in prototype of sdap_access_decide_offline,
> > which IMO doesn't need to return any value, since it returns alway EOK
> > now and there is no code which could be failing.
> 
> Nack
> 
> The change in child_common is wrong. The assignment of ret = errno is
> intentional, because ret was supposed to be used in the DEBUG statement
> below, rather than errno. It's supposed to be a protection in case the
> DEBUG macro ever changes to something that might reset errno internally.
> 
> So the real bug here is that we are printing errno instead of ret.

Thanks for the info.

>      if (be_is_offline(state->be_ctx)) {
>          /* Ok, we're offline. Return from the cache */
> -        ret = sdap_access_decide_offline(req);
> +        sdap_access_decide_offline(req);
>          goto finished;
>      }
> Please add ret = EOK before 'goto finished'. While it just happens to be
> true right now, I'd rather we kept this explicit here, in case we ever
> add something above this that changes ret.

That would create dead assignment and this patch should eliminate them rather 
than create new ones. I would agree with you if the ret value was used after 
"finished" label, but this isn't the case. I just want to be sure about this.

--
Jan



More information about the sssd-devel mailing list