[PATCH 5/7] nss_files: Add generic code for set*ent, end*ent and file open

Florian Weimer fweimer@redhat.com
Tue Jun 29 10:11:39 GMT 2021


This reduces RSS usage if nss_files is not actually used, and can
be used later to make NSS data thread-specific.  It also results in
a small code size reduction.

Before:

   text	   data	    bss	    dec	    hex	filename
   2288	      0	     72	   2360	    938	nss/files-alias.os
   1807	      0	     72	   1879	    757	nss/files-ethers.os
   1371	      0	     72	   1443	    5a3	nss/files-grp.os
   6246	      0	     72	   6318	   18ae	nss/files-hosts.os
    869	      0	      0	    869	    365	nss/files-initgroups.os
    666	      0	      0	    666	    29a	nss/files-init.os
   1934	      0	      0	   1934	    78e	nss/files-netgrp.os
   2353	      0	     72	   2425	    979	nss/files-network.os
   2130	      0	     72	   2202	    89a	nss/files-proto.os
   1372	      0	     72	   1444	    5a4	nss/files-pwd.os
   2124	      0	     72	   2196	    894	nss/files-rpc.os
   2265	      0	     72	   2337	    921	nss/files-service.os
   1125	      0	     72	   1197	    4ad	nss/files-sgrp.os
   1124	      0	     72	   1196	    4ac	nss/files-spwd.os



After:

   text	   data	    bss	    dec	    hex	filename
   2040	      0	      0	   2040	    7f8	nss/files-alias.os
   1599	      0	      0	   1599	    63f	nss/files-ethers.os
   1155	      0	      0	   1155	    483	nss/files-grp.os
   6010	      0	      0	   6010	   177a	nss/files-hosts.os
    869	      0	      0	    869	    365	nss/files-initgroups.os
    666	      0	      0	    666	    29a	nss/files-init.os
   1934	      0	      0	   1934	    78e	nss/files-netgrp.os
   2129	      0	      0	   2129	    851	nss/files-network.os
   1914	      0	      0	   1914	    77a	nss/files-proto.os
   1156	      0	      0	   1156	    484	nss/files-pwd.os
   1908	      0	      0	   1908	    774	nss/files-rpc.os
   2057	      0	      0	   2057	    809	nss/files-service.os
    909	      0	      0	    909	    38d	nss/files-sgrp.os
    908	      0	      0	    908	    38c	nss/files-spwd.os
   1090	      0	      8	   1098	    44a	nss/nss_files_data.os

27674 code bytes before, 26344 code bytes after, so it is an overall
win despite the extra initialization code.
---
 include/nss_files.h         |  60 ++++++++++++++
 nss/Makefile                |   2 +-
 nss/Versions                |   4 +
 nss/nss_files/files-XXX.c   |  67 ++++-----------
 nss/nss_files/files-alias.c |  69 ++++------------
 nss/nss_files/files-hosts.c |   4 +-
 nss/nss_files_data.c        | 159 ++++++++++++++++++++++++++++++++++++
 7 files changed, 257 insertions(+), 108 deletions(-)
 create mode 100644 nss/nss_files_data.c

diff --git a/include/nss_files.h b/include/nss_files.h
index 6a0dcdb85b..7bf1951496 100644
--- a/include/nss_files.h
+++ b/include/nss_files.h
@@ -20,6 +20,9 @@
 #define _NSS_FILES_H
 
 #include <stdio.h>
+#if IS_IN (libc)
+#include <libc-lock.h>
+#endif
 
 /* Open PATH for reading, as a data source for nss_files.  */
 FILE *__nss_files_fopen (const char *path);
@@ -47,6 +50,63 @@ int __nss_readline_seek (FILE *fp, off64_t offset) attribute_hidden;
 int __nss_parse_line_result (FILE *fp, off64_t offset, int parse_line_result);
 libc_hidden_proto (__nss_parse_line_result)
 
+/* Per-file data.  Used by the *ent functions that need to preserve
+   state across calls.  */
+struct nss_files_per_file_data
+{
+  FILE *stream;
+#if IS_IN (libc)
+  /* The size of locks changes between libc and nss_files, so this
+     member must be last and is only available in libc.  */
+  __libc_lock_define (, lock);
+#endif
+};
+
+/* File index for __nss_files_data_get.  */
+enum nss_files_file
+  {
+    nss_file_aliasent,
+    nss_file_etherent,
+    nss_file_grent,
+    nss_file_hostent,
+    nss_file_netent,
+    nss_file_protoent,
+    nss_file_pwent,
+    nss_file_rpcent,
+    nss_file_servent,
+    nss_file_sgent,
+    nss_file_spent,
+
+    nss_file_count
+  };
+
+/* Obtains a pointer to the per-file data for FILE, which is written
+   to *PDATA, and tries to open the file at PATH for it.  On success,
+   returns NSS_STATUS_SUCCESS, and the caller must later call
+   __nss_files_data_put.  On failure, NSS_STATUS_TRYAGAIN is returned,
+   and *ERRNOP and *HERRNOP are updated if these pointers are not
+   null.  */
+enum nss_status __nss_files_data_open (struct nss_files_per_file_data **pdata,
+                                       enum nss_files_file file,
+                                       const char *path,
+                                       int *errnop, int *herrnop);
+libc_hidden_proto (__nss_files_data_open)
+
+/* Unlock the per-file data, previously obtained by
+   __nss_files_data_open.  */
+void __nss_files_data_put (struct nss_files_per_file_data *data);
+libc_hidden_proto (__nss_files_data_put)
+
+/* Performs the set*ent operation for FILE.  PATH is the file to
+   open.  */
+enum nss_status __nss_files_data_setent (enum nss_files_file file,
+                                           const char *path);
+libc_hidden_proto (__nss_files_data_setent)
+
+/* Performs the end*ent operation for FILE.  */
+enum nss_status __nss_files_data_endent (enum nss_files_file file);
+libc_hidden_proto (__nss_files_data_endent)
+
 struct parser_data;
 
 /* Instances of the parse_line function from
diff --git a/nss/Makefile b/nss/Makefile
index 9682a31e20..271a0e7716 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -31,7 +31,7 @@ routines		= nsswitch getnssent getnssent_r digits_dots \
 			  compat-lookup nss_hash nss_files_fopen \
 			  nss_readline nss_parse_line_result \
 			  nss_fgetent_r nss_module nss_action \
-			  nss_action_parse nss_database
+			  nss_action_parse nss_database nss_files_data
 
 # These are the databases that go through nss dispatch.
 # Caution: if you add a database here, you must add its real name
diff --git a/nss/Versions b/nss/Versions
index fdddea104c..7b040b4786 100644
--- a/nss/Versions
+++ b/nss/Versions
@@ -19,6 +19,10 @@ libc {
     __nss_services_lookup2; __nss_next2; __nss_lookup;
     __nss_hash; __nss_database_get;
     __nss_files_fopen; __nss_readline; __nss_parse_line_result;
+    __nss_files_data_endent;
+    __nss_files_data_open;
+    __nss_files_data_put;
+    __nss_files_data_setent;
   }
 }
 
diff --git a/nss/nss_files/files-XXX.c b/nss/nss_files/files-XXX.c
index b4b989d9bb..91553d7ca5 100644
--- a/nss/nss_files/files-XXX.c
+++ b/nss/nss_files/files-XXX.c
@@ -45,10 +45,12 @@
 # include <netdb.h>
 # define H_ERRNO_PROTO	, int *herrnop
 # define H_ERRNO_ARG	, herrnop
+# define H_ERRNO_ARG_OR_NULL herrnop
 # define H_ERRNO_SET(val) (*herrnop = (val))
 #else
 # define H_ERRNO_PROTO
 # define H_ERRNO_ARG
+# define H_ERRNO_ARG_OR_NULL NULL
 # define H_ERRNO_SET(val) ((void) 0)
 #endif
 
@@ -58,15 +60,11 @@
 # define EXTRA_ARGS_VALUE
 #endif
 
-/* Locks the static variables in this file.  */
-__libc_lock_define_initialized (static, lock)
 
 /* Maintenance of the stream open on the database file.  For getXXent
    operations the stream needs to be held open across calls, the other
    getXXbyYY operations all use their own stream.  */
 
-static FILE *stream;
-
 /* Open database file if not already opened.  */
 static enum nss_status
 internal_setent (FILE **stream)
@@ -91,41 +89,13 @@ internal_setent (FILE **stream)
 enum nss_status
 CONCAT(_nss_files_set,ENTNAME) (int stayopen)
 {
-  enum nss_status status;
-
-  __libc_lock_lock (lock);
-
-  status = internal_setent (&stream);
-
-  __libc_lock_unlock (lock);
-
-  return status;
-}
-
-
-/* Close the database file.  */
-static void
-internal_endent (FILE **stream)
-{
-  if (*stream != NULL)
-    {
-      fclose (*stream);
-      *stream = NULL;
-    }
+  return __nss_files_data_setent (CONCAT (nss_file_, ENTNAME), DATAFILE);
 }
 
-
-/* Thread-safe, exported version of that.  */
 enum nss_status
 CONCAT(_nss_files_end,ENTNAME) (void)
 {
-  __libc_lock_lock (lock);
-
-  internal_endent (&stream);
-
-  __libc_lock_unlock (lock);
-
-  return NSS_STATUS_SUCCESS;
+  return __nss_files_data_endent (CONCAT (nss_file_, ENTNAME));
 }
 
 
@@ -194,26 +164,19 @@ CONCAT(_nss_files_get,ENTNAME_r) (struct STRUCTURE *result, char *buffer,
 				  size_t buflen, int *errnop H_ERRNO_PROTO)
 {
   /* Return next entry in host file.  */
-  enum nss_status status = NSS_STATUS_SUCCESS;
-
-  __libc_lock_lock (lock);
-
-  /* Be prepared that the set*ent function was not called before.  */
-  if (stream == NULL)
-    {
-      int save_errno = errno;
-
-      status = internal_setent (&stream);
-
-      __set_errno (save_errno);
-    }
 
-  if (status == NSS_STATUS_SUCCESS)
-    status = internal_getent (stream, result, buffer, buflen, errnop
-			      H_ERRNO_ARG EXTRA_ARGS_VALUE);
+  struct nss_files_per_file_data *data;
+  enum nss_status status = __nss_files_data_open (&data,
+						  CONCAT (nss_file_, ENTNAME),
+						  DATAFILE,
+						  errnop, H_ERRNO_ARG_OR_NULL);
+  if (status != NSS_STATUS_SUCCESS)
+    return status;
 
-  __libc_lock_unlock (lock);
+  status = internal_getent (data->stream, result, buffer, buflen, errnop
+			    H_ERRNO_ARG EXTRA_ARGS_VALUE);
 
+  __nss_files_data_put (data);
   return status;
 }
 
@@ -248,7 +211,7 @@ _nss_files_get##name##_r (proto,					      \
 	     == NSS_STATUS_SUCCESS)					      \
 	{ break_if_match }						      \
 									      \
-      internal_endent (&stream);					      \
+      fclose (stream);							      \
     }									      \
 									      \
   return status;							      \
diff --git a/nss/nss_files/files-alias.c b/nss/nss_files/files-alias.c
index 30971bfe56..9624b6224c 100644
--- a/nss/nss_files/files-alias.c
+++ b/nss/nss_files/files-alias.c
@@ -33,16 +33,11 @@
 
 NSS_DECLARE_MODULE_FUNCTIONS (files)
 
-/* Locks the static variables in this file.  */
-__libc_lock_define_initialized (static, lock)
 
 /* Maintenance of the stream open on the database file.  For getXXent
    operations the stream needs to be held open across calls, the other
    getXXbyYY operations all use their own stream.  */
 
-static FILE *stream;
-
-
 static enum nss_status
 internal_setent (FILE **stream)
 {
@@ -66,41 +61,13 @@ internal_setent (FILE **stream)
 enum nss_status
 _nss_files_setaliasent (void)
 {
-  enum nss_status status;
-
-  __libc_lock_lock (lock);
-
-  status = internal_setent (&stream);
-
-  __libc_lock_unlock (lock);
-
-  return status;
-}
-
-
-/* Close the database file.  */
-static void
-internal_endent (FILE **stream)
-{
-  if (*stream != NULL)
-    {
-      fclose (*stream);
-      *stream = NULL;
-    }
+  return __nss_files_data_setent (nss_file_aliasent, "/etc/aliases");
 }
 
-
-/* Thread-safe, exported version of that.  */
 enum nss_status
 _nss_files_endaliasent (void)
 {
-  __libc_lock_lock (lock);
-
-  internal_endent (&stream);
-
-  __libc_lock_unlock (lock);
-
-  return NSS_STATUS_SUCCESS;
+  return __nss_files_data_endent (nss_file_aliasent);
 }
 
 /* Parsing the database file into `struct aliasent' data structures.  */
@@ -369,26 +336,22 @@ _nss_files_getaliasent_r (struct aliasent *result, char *buffer, size_t buflen,
 			  int *errnop)
 {
   /* Return next entry in host file.  */
-  enum nss_status status = NSS_STATUS_SUCCESS;
 
-  __libc_lock_lock (lock);
+  struct nss_files_per_file_data *data;
+  enum nss_status status = __nss_files_data_open (&data, nss_file_aliasent,
+						  "/etc/aliases", errnop, NULL);
+  if (status != NSS_STATUS_SUCCESS)
+    return status;
 
-  /* Be prepared that the set*ent function was not called before.  */
-  if (stream == NULL)
-    status = internal_setent (&stream);
+  result->alias_local = 1;
 
-  if (status == NSS_STATUS_SUCCESS)
-    {
-      result->alias_local = 1;
-
-      /* Read lines until we get a definite result.  */
-      do
-	status = get_next_alias (stream, NULL, result, buffer, buflen, errnop);
-      while (status == NSS_STATUS_RETURN);
-    }
-
-  __libc_lock_unlock (lock);
+  /* Read lines until we get a definite result.  */
+  do
+    status = get_next_alias (data->stream, NULL, result, buffer, buflen,
+			     errnop);
+  while (status == NSS_STATUS_RETURN);
 
+  __nss_files_data_put (data);
   return status;
 }
 
@@ -418,9 +381,9 @@ _nss_files_getaliasbyname_r (const char *name, struct aliasent *result,
       do
 	status = get_next_alias (stream, name, result, buffer, buflen, errnop);
       while (status == NSS_STATUS_RETURN);
-    }
 
-  internal_endent (&stream);
+      fclose (stream);
+    }
 
   return status;
 }
diff --git a/nss/nss_files/files-hosts.c b/nss/nss_files/files-hosts.c
index 2b47ec3e53..1dd51d1db9 100644
--- a/nss/nss_files/files-hosts.c
+++ b/nss/nss_files/files-hosts.c
@@ -349,7 +349,7 @@ _nss_files_gethostbyname3_r (const char *name, int af, struct hostent *result,
 	status = gethostbyname3_multi
 	  (stream, name, af, result, buffer, buflen, errnop, herrnop);
 
-      internal_endent (&stream);
+      fclose (stream);
     }
 
   if (canonp && status == NSS_STATUS_SUCCESS)
@@ -475,7 +475,7 @@ _nss_files_gethostbyname4_r (const char *name, struct gaih_addrtuple **pat,
 	  status = NSS_STATUS_SUCCESS;
 	}
 
-      internal_endent (&stream);
+      fclose (stream);
     }
   else if (status == NSS_STATUS_TRYAGAIN)
     {
diff --git a/nss/nss_files_data.c b/nss/nss_files_data.c
new file mode 100644
index 0000000000..7b7662abf1
--- /dev/null
+++ b/nss/nss_files_data.c
@@ -0,0 +1,159 @@
+/* Returns a pointer to the global nss_files data structure.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <nss_files.h>
+
+#include <allocate_once.h>
+#include <errno.h>
+#include <netdb.h>
+#include <nss.h>
+#include <stdlib.h>
+
+/* This collects all per file-data.   */
+struct nss_files_data
+{
+  struct nss_files_per_file_data files[nss_file_count];
+};
+
+/* For use with allocate_once.  */
+static void *nss_files_global;
+static void *
+nss_files_global_allocate (void *closure)
+{
+  struct nss_files_data *result = malloc (sizeof (*result));
+  if (result != NULL)
+    for (int i = 0; i < nss_file_count; ++i)
+      {
+        result->files[i].stream = NULL;
+        __libc_lock_init (result->files[i].lock);
+      }
+  return result;
+}
+/* Like __nss_files_data_open, but does not perform the open call.  */
+static enum nss_status
+__nss_files_data_get (struct nss_files_per_file_data **pdata,
+                      enum nss_files_file file, int *errnop, int *herrnop)
+{
+  struct nss_files_data *data = allocate_once (&nss_files_global,
+                                               nss_files_global_allocate,
+                                               NULL, NULL);
+  if (data == NULL)
+    {
+      if (errnop != NULL)
+        *errnop = errno;
+      if (herrnop != NULL)
+        {
+          __set_h_errno (NETDB_INTERNAL);
+          *herrnop = NETDB_INTERNAL;
+        }
+      return NSS_STATUS_TRYAGAIN;
+    }
+
+  *pdata = &data->files[file];
+  __libc_lock_lock ((*pdata)->lock);
+  return NSS_STATUS_SUCCESS;
+}
+
+/* Helper function for opening the backing file at PATH.  */
+static enum nss_status
+__nss_files_data_internal_open (struct nss_files_per_file_data *data,
+                                const char *path)
+{
+  enum nss_status status = NSS_STATUS_SUCCESS;
+
+  if (data->stream == NULL)
+    {
+      data->stream = __nss_files_fopen (path);
+
+      if (data->stream == NULL)
+        status = errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
+    }
+
+  return status;
+}
+
+
+enum nss_status
+__nss_files_data_open (struct nss_files_per_file_data **pdata,
+                       enum nss_files_file file, const char *path,
+                       int *errnop, int *herrnop)
+{
+  enum nss_status status = __nss_files_data_get (pdata, file, errnop, herrnop);
+  if (status != NSS_STATUS_SUCCESS)
+    return status;
+
+  /* Be prepared that the set*ent function was not called before.  */
+  if ((*pdata)->stream == NULL)
+    {
+      int saved_errno = errno;
+      status = __nss_files_data_internal_open (*pdata, path);
+      __set_errno (saved_errno);
+      if (status != NSS_STATUS_SUCCESS)
+        __nss_files_data_put (*pdata);
+    }
+
+  return status;
+}
+
+libc_hidden_def (__nss_files_data_open)
+
+void
+__nss_files_data_put (struct nss_files_per_file_data *data)
+{
+  __libc_lock_unlock (data->lock);
+}
+libc_hidden_def (__nss_files_data_put)
+
+enum nss_status
+__nss_files_data_setent (enum nss_files_file file, const char *path)
+{
+  struct nss_files_per_file_data *data;
+  enum nss_status status = __nss_files_data_get (&data, file, NULL, NULL);
+  if (status != NSS_STATUS_SUCCESS)
+    return status;
+
+  if (data->stream == NULL)
+    status = __nss_files_data_internal_open (data, path);
+  else
+    rewind (data->stream);
+
+  __nss_files_data_put (data);
+  return status;
+}
+libc_hidden_def (__nss_files_data_setent)
+
+enum nss_status
+__nss_files_data_endent (enum nss_files_file file)
+{
+  /* No cleanup is necessary if not initialized.  */
+  struct nss_files_data *data = atomic_load_acquire (&nss_files_global);
+  if (data == NULL)
+    return NSS_STATUS_SUCCESS;
+
+  struct nss_files_per_file_data *fdata = &data->files[file];
+  __libc_lock_lock (fdata->lock);
+  if (fdata->stream != NULL)
+    {
+      fclose (fdata->stream);
+      fdata->stream = NULL;
+    }
+  __libc_lock_unlock (fdata->lock);
+
+  return NSS_STATUS_SUCCESS;
+}
+libc_hidden_def (__nss_files_data_endent)
-- 
2.31.1




More information about the Libc-alpha mailing list