diff mbox

[roland/nscd-vlais] Rework some nscd code not to use variable-length struct types.

Message ID 20141022212008.67EC42C3ABF@topped-with-meat.com
State Committed
Headers show

Commit Message

Roland McGrath Oct. 22, 2014, 9:20 p.m. UTC
This is pure cleanup, for use of a GNU C extension that really wasn't
making things any better than sticking to C99 features.  

With GCC 4.8.2 on x86_64-linux-gnu, this perturbed code generation more
than I would have expected.  But all signs are that the new code is at
least as good.  It's slightly smaller in each case, for some reason.
Most of the differences I saw seemed to be the boring stuff like
different register allocation decisions, but it was different enough
that it was not straightforward to account for every difference and I
did not try very hard.

This caused no regressions on x86_64-linux-gnu, but we don't have any
tests for nscd so that doesn't really say much.  

I'd appreciate a second pair of eyes on this, but if nobody says
anything at all I'll commit it in a couple of days.


Thanks,
Roland


2014-10-22  Roland McGrath  <roland@hack.frob.com>

	* inet/netgroup.h (struct name_list): Use C99 [] syntax rather than
	old GNU extension [0] syntax.
	* nscd/nscd_helper.c (open_socket): Use a flexible array member and
	alloca rather than an array member with variable length.
	* nscd/netgroupcache.c (addgetnetgrentX): Likewise.
	* nscd/nscd.c (invalidate_db): New function, broken out of ...
	(parse_opt): ... here.  Likewise use alloca there.
	Validate the -i argument before checking for rootness.
	(send_shutdown): New function, broken out of ...
	(parse_opt): ... here.

Comments

Andreas Schwab Oct. 22, 2014, 10:01 p.m. UTC | #1
Roland McGrath <roland@hack.frob.com> writes:

> 	* inet/netgroup.h (struct name_list): Use C99 [] syntax rather than
> 	old GNU extension [0] syntax.
> 	* nscd/nscd_helper.c (open_socket): Use a flexible array member and
> 	alloca rather than an array member with variable length.
> 	* nscd/netgroupcache.c (addgetnetgrentX): Likewise.
> 	* nscd/nscd.c (invalidate_db): New function, broken out of ...
> 	(parse_opt): ... here.  Likewise use alloca there.
> 	Validate the -i argument before checking for rootness.
> 	(send_shutdown): New function, broken out of ...
> 	(parse_opt): ... here.

Looks good.

Andreas.
Roland McGrath Oct. 22, 2014, 10:04 p.m. UTC | #2
Committed.

Thanks,
Roland
diff mbox

Patch

--- a/inet/netgroup.h
+++ b/inet/netgroup.h
@@ -26,7 +26,7 @@ 
 struct name_list
 {
   struct name_list *next;
-  char name[0];
+  char name[];
 };
 
 
--- a/nscd/netgroupcache.c
+++ b/nscd/netgroupcache.c
@@ -21,6 +21,7 @@ 
 #include <errno.h>
 #include <libintl.h>
 #include <stdbool.h>
+#include <stdlib.h>
 #include <unistd.h>
 #include <sys/mman.h>
 
@@ -136,11 +137,8 @@  addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
   char *buffer = NULL;
   size_t nentries = 0;
   size_t group_len = strlen (key) + 1;
-  union
-  {
-    struct name_list elem;
-    char mem[sizeof (struct name_list) + group_len];
-  } first_needed;
+  struct name_list *first_needed
+    = alloca (sizeof (struct name_list) + group_len);
 
   if (netgroup_database == NULL
       && __nss_database_lookup ("netgroup", NULL, NULL, &netgroup_database))
@@ -153,9 +151,9 @@  addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
 
   memset (&data, '\0', sizeof (data));
   buffer = xmalloc (buflen);
-  first_needed.elem.next = &first_needed.elem;
-  memcpy (first_needed.elem.name, key, group_len);
-  data.needed_groups = &first_needed.elem;
+  first_needed->next = first_needed;
+  memcpy (first_needed->name, key, group_len);
+  data.needed_groups = first_needed;
 
   while (data.needed_groups != NULL)
     {
--- a/nscd/nscd.c
+++ b/nscd/nscd.c
@@ -324,6 +324,75 @@  main (int argc, char **argv)
 }
 
 
+static void __attribute__ ((noreturn))
+invalidate_db (const char *dbname)
+{
+  int sock = nscd_open_socket ();
+
+  if (sock == -1)
+    exit (EXIT_FAILURE);
+
+  size_t dbname_len = strlen (dbname) + 1;
+  size_t reqlen = sizeof (request_header) + dbname_len;
+  struct
+  {
+    request_header req;
+    char dbname[];
+  } *reqdata = alloca (reqlen);
+
+  reqdata->req.key_len = dbname_len;
+  reqdata->req.version = NSCD_VERSION;
+  reqdata->req.type = INVALIDATE;
+  memcpy (reqdata->dbname, dbname, dbname_len);
+
+  ssize_t nbytes = TEMP_FAILURE_RETRY (send (sock, reqdata, reqlen,
+					     MSG_NOSIGNAL));
+
+  if (nbytes != reqlen)
+    {
+      int err = errno;
+      close (sock);
+      error (EXIT_FAILURE, err, _("write incomplete"));
+    }
+
+  /* Wait for ack.  Older nscd just closed the socket when
+     prune_cache finished, silently ignore that.  */
+  int32_t resp = 0;
+  nbytes = TEMP_FAILURE_RETRY (read (sock, &resp, sizeof (resp)));
+  if (nbytes != 0 && nbytes != sizeof (resp))
+    {
+      int err = errno;
+      close (sock);
+      error (EXIT_FAILURE, err, _("cannot read invalidate ACK"));
+    }
+
+  close (sock);
+
+  if (resp != 0)
+    error (EXIT_FAILURE, resp, _("invalidation failed"));
+
+  exit (0);
+}
+
+static void __attribute__ ((noreturn))
+send_shutdown (void)
+{
+  int sock = nscd_open_socket ();
+
+  if (sock == -1)
+    exit (EXIT_FAILURE);
+
+  request_header req;
+  req.version = NSCD_VERSION;
+  req.type = SHUTDOWN;
+  req.key_len = 0;
+
+  ssize_t nbytes = TEMP_FAILURE_RETRY (send (sock, &req, sizeof req,
+                                             MSG_NOSIGNAL));
+  close (sock);
+  exit (nbytes != sizeof (request_header) ? EXIT_FAILURE : EXIT_SUCCESS);
+}
+
 /* Handle program arguments.  */
 static error_t
 parse_opt (int key, char *arg, struct argp_state *state)
@@ -346,91 +415,34 @@  parse_opt (int key, char *arg, struct argp_state *state)
     case 'K':
       if (getuid () != 0)
 	error (4, 0, _("Only root is allowed to use this option!"));
-      {
-	int sock = nscd_open_socket ();
-
-	if (sock == -1)
-	  exit (EXIT_FAILURE);
-
-	request_header req;
-	req.version = NSCD_VERSION;
-	req.type = SHUTDOWN;
-	req.key_len = 0;
-
-	ssize_t nbytes = TEMP_FAILURE_RETRY (send (sock, &req,
-						   sizeof (request_header),
-						   MSG_NOSIGNAL));
-	close (sock);
-	exit (nbytes != sizeof (request_header) ? EXIT_FAILURE : EXIT_SUCCESS);
-      }
+      else
+        send_shutdown ();
+      break;
 
     case 'g':
       get_stats = true;
       break;
 
     case 'i':
+      {
+        /* Validate the database name.  */
+
+        dbtype cnt;
+        for (cnt = pwddb; cnt < lastdb; ++cnt)
+          if (strcmp (arg, dbnames[cnt]) == 0)
+            break;
+
+        if (cnt == lastdb)
+          {
+            argp_error (state, _("'%s' is not a known database"), arg);
+            return EINVAL;
+          }
+      }
       if (getuid () != 0)
 	error (4, 0, _("Only root is allowed to use this option!"));
       else
-	{
-	  int sock = nscd_open_socket ();
-
-	  if (sock == -1)
-	    exit (EXIT_FAILURE);
-
-	  dbtype cnt;
-	  for (cnt = pwddb; cnt < lastdb; ++cnt)
-	    if (strcmp (arg, dbnames[cnt]) == 0)
-	      break;
-
-	  if (cnt == lastdb)
-	    {
-	      argp_error (state, _("'%s' is not a known database"), arg);
-	      return EINVAL;
-	    }
-
-	  size_t arg_len = strlen (arg) + 1;
-	  struct
-	  {
-	    request_header req;
-	    char arg[arg_len];
-	  } reqdata;
-
-	  reqdata.req.key_len = strlen (arg) + 1;
-	  reqdata.req.version = NSCD_VERSION;
-	  reqdata.req.type = INVALIDATE;
-	  memcpy (reqdata.arg, arg, arg_len);
-
-	  ssize_t nbytes = TEMP_FAILURE_RETRY (send (sock, &reqdata,
-						     sizeof (request_header)
-						     + arg_len,
-						     MSG_NOSIGNAL));
-
-	  if (nbytes != sizeof (request_header) + arg_len)
-	    {
-	      int err = errno;
-	      close (sock);
-	      error (EXIT_FAILURE, err, _("write incomplete"));
-	    }
-
-	  /* Wait for ack.  Older nscd just closed the socket when
-	     prune_cache finished, silently ignore that.  */
-	  int32_t resp = 0;
-	  nbytes = TEMP_FAILURE_RETRY (read (sock, &resp, sizeof (resp)));
-	  if (nbytes != 0 && nbytes != sizeof (resp))
-	    {
-	      int err = errno;
-	      close (sock);
-	      error (EXIT_FAILURE, err, _("cannot read invalidate ACK"));
-	    }
-
-	  close (sock);
-
-	  if (resp != 0)
-	    error (EXIT_FAILURE, resp, _("invalidation failed"));
-
-	  exit (0);
-	}
+        invalidate_db (arg);
+      break;
 
     case 't':
       nthreads = atol (arg);
--- a/nscd/nscd_helper.c
+++ b/nscd/nscd_helper.c
@@ -21,6 +21,7 @@ 
 #include <fcntl.h>
 #include <stdbool.h>
 #include <stddef.h>
+#include <stdlib.h>
 #include <string.h>
 #include <time.h>
 #include <unistd.h>
@@ -186,12 +187,12 @@  open_socket (request_type type, const char *key, size_t keylen)
   if (sock < 0)
     return -1;
 
+  size_t real_sizeof_reqdata = sizeof (request_header) + keylen;
   struct
   {
     request_header req;
-    char key[keylen];
-  } reqdata;
-  size_t real_sizeof_reqdata = sizeof (request_header) + keylen;
+    char key[];
+  } *reqdata = alloca (real_sizeof_reqdata);
 
 #ifndef __ASSUME_SOCK_CLOEXEC
 # ifdef SOCK_NONBLOCK
@@ -208,11 +209,11 @@  open_socket (request_type type, const char *key, size_t keylen)
       && errno != EINPROGRESS)
     goto out;
 
-  reqdata.req.version = NSCD_VERSION;
-  reqdata.req.type = type;
-  reqdata.req.key_len = keylen;
+  reqdata->req.version = NSCD_VERSION;
+  reqdata->req.type = type;
+  reqdata->req.key_len = keylen;
 
-  memcpy (reqdata.key, key, keylen);
+  memcpy (reqdata->key, key, keylen);
 
   bool first_try = true;
   struct timeval tvend;
@@ -223,7 +224,7 @@  open_socket (request_type type, const char *key, size_t keylen)
 #ifndef MSG_NOSIGNAL
 # define MSG_NOSIGNAL 0
 #endif
-      ssize_t wres = TEMP_FAILURE_RETRY (__send (sock, &reqdata,
+      ssize_t wres = TEMP_FAILURE_RETRY (__send (sock, reqdata,
 						 real_sizeof_reqdata,
 						 MSG_NOSIGNAL));
       if (__glibc_likely (wres == (ssize_t) real_sizeof_reqdata))