[Freeipa-devel] [PATCH][SSSD] Implement GetUserAttributes in the InfoPipe

[Simo Sorce]

On Mon, 2009-03-02 at 11:43 -0500, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Simo Sorce wrote:
> > On Mon, 2009-03-02 at 09:16 -0500, Stephen Gallagher wrote:
> >> -----BEGIN PGP SIGNED MESSAGE-----
> >> Hash: SHA1
> >>
> >> Simo Sorce wrote:
> >>> On Mon, 2009-03-02 at 08:56 -0500, Stephen Gallagher wrote:
> >>>> You can't use strdup, because btreemap takes a void *. This is
> >>>> necessary
> >>>> so it can hold arbitrary data (see its use in
> >>>> create_getattr_result_map).
> >>> If thekeys are void we can turn them into a char *, are we ever going to
> >>> use keys that are not strings?
> >> Probably not, but I wanted to leave it expandable in case we discovered
> >> such a case.
> >>
> >>>> Also, I wasn't using talloc_reference, it's talloc_steal. Yes, I know
> >>>> that's not ideal either. I'm open to suggestions.
> >>> talloc_steal() is much worse in this case.
> >>>
> >> Can you explain to me what, exactly, is dangerous about
> >> talloc_reference? It seems to me that calling
> >> talloc_reference(btreemap_node, btreemap_value) would solve this handily.
> > 
> > The poroblem is the change in ownership when you free the original
> > owner. It makes it very simple to loose track of who owns what, causing
> > potential unintended consequences. The semantics are still not very
> > clear imo, and they may change upstream at some point, so I prefer to
> > simply not use it for now.
> > 
> > Simo.
> > 
> 
> Well, in this particular case, my whole argument is that we want to make
> sure that the btreemap is always valid, even if the original owner has
> gone away. I think if we just added a few verbose comments to the
> btreemap.h to state that the map acquires ownership of anything passed
> into it, it might be acceptable.

I prefer to make it very clear that the owner of the map must own the
contents as well.
The map is useful only for its owner after all, I don't see a case where
it would be useful for a map to survive its owner.

> Otherwise, we have to be worried about the original context disappearing.

Given the btreemap should be allocated by the owner, if it disappears it
does not matter as the btreemap will be freed as well in cascade.

> I understand the wariness to lose track of ownership, but I think this
> would be a very carefully controlled usage.

patch coming :)

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York