[V2] Enhance comments in nsswitch.h

Message ID xnbm04d8si.fsf@greed.delorie.com
State New, archived
Headers

Commit Message

DJ Delorie May 14, 2019, 9:29 p.m. UTC
  "Carlos O'Donell" <codonell@redhat.com> writes:
> Please post a v2. I'd like to see this committed.

V2 it is...

From 7696eca99b0f74b81dfb5af5848194db34ad1b4a Mon Sep 17 00:00:00 2001
From: DJ Delorie <dj@redhat.com>
Date: Tue, 14 May 2019 17:25:12 -0400
Subject: Document nsswitch.h more.

Add more comments to structures in nsswitch.conf to document their
current use, and potential reloadability.
  

Comments

Florian Weimer May 15, 2019, 6:01 a.m. UTC | #1
* DJ Delorie:

> +   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?

Is ptr-to-persistent to a static object?

Thanks,
Florian
  

Patch

diff --git a/ChangeLog b/ChangeLog
index cfdfcd57b7..0e3e2dd779 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@ 
+2019-05-14  DJ Delorie  <dj@redhat.com>
+
+	* nss/nsswitch.h: Add more comments.
+
 2019-05-14  Florian Weimer  <fweimer@redhat.com>
 
 	Linux: Add the tgkill function.
diff --git a/nss/nsswitch.h b/nss/nsswitch.h
index 475e007e33..756db903b2 100644
--- a/nss/nsswitch.h
+++ b/nss/nsswitch.h
@@ -36,6 +36,19 @@  typedef enum
   NSS_ACTION_MERGE
 } lookup_actions;
 
+/* The NSS implementation uses a variety of data structures to
+   implement the service plugin management.  Some of this data
+   once loaded must be persistent.  Some of this data once loaded
+   could subsequently be unloaded.  These comments are here to help
+   guide a future implementation where we might unload this data.
+
+   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.  */
+
+/* The following structures generally hold persistent data
+   that cannot (or at least need not) be unloaded:  */
 
 typedef struct service_library
 {
@@ -43,7 +56,9 @@  typedef struct service_library
   const char *name;
   /* Pointer to the loaded shared library.  */
   void *lib_handle;
-  /* And the link to the next entry.  */
+  /* The link to the next entry, set by nss_new_service.  This link is
+     used to free resources, and is not related to the order of
+     handlers for each service (that is NEXT in service_user).  */
   struct service_library *next;
 } service_library;
 
@@ -53,20 +68,28 @@  typedef struct service_library
    is the first member.  */
 typedef struct
 {
-  const char *fct_name;
-  void *fct_ptr;
+  /* This is the API name, like `gethostbyname_r'.  */
+  const char *fct_name; /* ptr-to-malloc'd */
+  /* This is the DSO function, like _nss_dns_gethostbyname_r().  */
+  void *fct_ptr; /* ptr-to-persistent */
 } known_function;
 
+/* The following structures generally hold data that may be unloaded
+   at runtime and/or replaced, such as if nsswitch.conf is
+   reloaded: */
 
 typedef struct service_user
 {
-  /* And the link to the next entry.  */
-  struct service_user *next;
+  /* The link to the next entry.  In a line like `a : b c d' this is
+     the b->c->d link.  */
+  struct service_user *next; /* ptr-to-unloadable */
   /* Action according to result.  */
   lookup_actions actions[5];
   /* Link to the underlying library object.  */
-  service_library *library;
-  /* Collection of known functions.  */
+  service_library *library; /* ptr-to-persistent */
+  /* Collection of known functions, mapping (for example) `foo' to
+     _nss_dns_foo().  Only contains mappings for this service and this
+     DSO, so at least one and at most a few mappings.  */
   void *known;
   /* Name of the service (`files', `dns', `nis', ...).  */
   char name[0];
@@ -78,11 +101,13 @@  typedef struct service_user
 
 typedef struct name_database_entry
 {
-  /* And the link to the next entry.  */
-  struct name_database_entry *next;
-  /* List of service to be used.  */
-  service_user *service;
-  /* Name of the database.  */
+  /* The link to the next entry.  In a typical nsswitch.conf, this is
+     the passwd->group->hosts link, i.e. the next non-comment line in
+     the file.  */
+  struct name_database_entry *next; /* ptr-to-unloadable */
+  /* List of services to be used.  */
+  service_user *service; /* ptr-to-unloadable */
+  /* Name of the database (`passwd', `hosts', etc).  */
   char name[0];
 } name_database_entry;
 
@@ -90,9 +115,9 @@  typedef struct name_database_entry
 typedef struct name_database
 {
   /* List of all known databases.  */
-  name_database_entry *entry;
+  name_database_entry *entry; /* ptr-to-unloadable */
   /* List of libraries with service implementation.  */
-  service_library *library;
+  service_library *library; /* ptr-to-persistent */
 } name_database;