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.