[SSSD] [PATCH] Remove useless assignment to function parameter

Lukas Slebodnik lslebodn at redhat.com
Mon Mar 2 13:43:24 UTC 2015


On (02/03/15 14:39), Sumit Bose wrote:
>On Mon, Mar 02, 2015 at 11:27:09AM +0100, Sumit Bose wrote:
>> On Mon, Mar 02, 2015 at 10:43:36AM +0100, Jakub Hrozek wrote:
>> > On Mon, Mar 02, 2015 at 10:41:22AM +0100, Pavel Reichl wrote:
>> > > 
>> > > On 03/02/2015 10:38 AM, Jakub Hrozek wrote:
>> > > >On Mon, Mar 02, 2015 at 10:29:00AM +0100, Pavel Reichl wrote:
>> > > >>On 03/02/2015 10:02 AM, Lukas Slebodnik wrote:
>> > > >>>On (28/02/15 22:24), Pavel Reichl wrote:
>> > > >>>>On 02/27/2015 05:27 PM, Lukas Slebodnik wrote:
>> > > >>>>>ehlo,
>> > > >>>>>
>> > > >>>>>I found this patch in my old branches. It still applies to sssd and remove dead
>> > > >>>>>code.
>> > > >>>>>
>> > > >>>>>LS
>> > > >>>>>
>> > > >>>>>
>> > > >>>>Hello Lukas,
>> > > >>>>
>> > > >>>>what I don't like about your patch is the fact that you are changing the
>> > > >>>>intentions of the author of the original code.
>> > > >>>>
>> > > >>>I did not change anything. It is dead code and it's very likely compiler
>> > > >>>optimize it out anyway.
>> > > >>>
>> > > >>>There is a BIG difference between intention of author and what code does :-)
>> > > >>Yes, I'm well aware of that.
>> > > >>>>Still there's no way how to fix function to do what was intended because we
>> > > >>>>can't change the type of the parameter. At least not for
>> > > >>>>sssd_krb5_locator_close() and free_exp_data().
>> > > >>>>
>> > > >>>>I'm not sure whether we can modify definition of hbac_free_info(). Do you
>> > > >>>>know?
>> > > >>>>
>> > > >>>We cannot change API of hbac_free_info either. It is part of public API
>> > > >>>$ nm --defined-only --dynamic /usr/lib64/libipa_hbac.so | grep free
>> > > >>>0000000000001030 T hbac_free_info
>> > > >>OK, thanks.
>> > > >>
>> > > >>ACK (ci passed:http://sssd-ci.duckdns.org/logs/job/8/43/summary.html)
>> > > >Did either of you reach out to the original authors to see if they're OK
>> > > >with the removal? If not, can you?
>> > > OK, I'll do it, but I'm not sure if one of the authors is Yassir.
>> > 
>> > Stephen wrote the HBAC evaluator, Sumit created both the krb5 plugin and
>> > pam_sss.
>> 
>> Hi,
>> 
>> since both the krb5 locator plugin and pam_sss are loaded into other
>> programs I wanted to make sure to clean up as much as possible to avoid
>> issues in the caller even if the caller misbehaves.
>> 
>> Although I see the point that this might hide issues in the caller I
>> think it would makes sense to stay defensive here.
>
>I'm sorry, Pavel pointed out to me that we only clear the local copy of
>the pointer and not the origin (we only get * not **). In this case the
>assignment is useless.
>
That's exactly reason why cppcheck reported it as dead assignment.

LS



More information about the sssd-devel mailing list