Re: [V2 patch] Enhance comments in nsswitch.h

[Florian Weimer <fweimer at redhat dot com>]

* DJ Delorie:

> Florian Weimer <fweimer@redhat.com> writes:
>>> +   We define several types of pointers to try help identify the data:
>>> +   - ptr-to-malloc'd: Pointer to data which needs freeing.
>>> +   - ptr-to-persistent: Pointer to persistent object.
>>> +   - ptr-to-unloadable: Pointer to unloadlable object.  */
>>
>> What is a pointer to an unloadable object?  A DSO handle?  Or is it a
>> heap pointer that is never freed?
>
> The intention is, if/when we allow for nsswitch.conf to be reloaded, we
> need to understand which internal data *is* reloaded, and which *isn't*.
>
> So ptr-to-unloadable means those malloc'd objects that can be free'd
> when we reload nsswitch.conf.

But that doesn't seem true for e.g. service_user objects in the current
framework because threads traverse the list without locking.  We cannot
trivially add locking because we keep pointers service_user objects
across calls to NSS module functions, which can block for a long time.
Furthermore, nss_load_library writes a pointer the name from the
service_user object into the persistent service_library object, and
that's another obstacle to deallocation.

(service_user is even part of the ABI, but I'll try to come up with a
patch to change that.  unscd doesn't __nss_database_lookup which exposed
it.)

>> Is ptr-to-persistent to a static object?
>
> Just to something that doesn't change once created.  The problem is, if
> we load a DSO and some thread is using it via nss, and we reload
> nsswitch.conf, we can't unload the DSO, because it might be in use.

That's true for all data structures right now.  However, we shouldn't
dlclose any DSOs because they might not be prepared to be unloaded.

> So ptr-to-persistent means it points to something that, once created,
> won't go away.  Since the DSO doesn't change, any data mapping names
> to DSO functions also won't need to change, etc.

Okay.

I think the high-level question for me is whether this
can-be-deallocated property is a property of the struct member (where
you documented it in the patch), or a property of the type.  For
example, it seems to me that all name_database_entry objects can be
reloaded.  Maybe there is value in keeping this simple and not share
data too much?

Thanks,
Florian