[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