From patchwork Wed Oct 22 21:20:08 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roland McGrath X-Patchwork-Id: 3327 Received: (qmail 28797 invoked by alias); 22 Oct 2014 21:20:13 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 28785 invoked by uid 89); 22 Oct 2014 21:20:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL, BAYES_00 autolearn=ham version=3.3.2 X-HELO: topped-with-meat.com MIME-Version: 1.0 From: Roland McGrath To: "GNU C. Library" CC: konstantin.s.serebryany@gmail.com Subject: [PATCH roland/nscd-vlais] Rework some nscd code not to use variable-length struct types. Message-Id: <20141022212008.67EC42C3ABF@topped-with-meat.com> Date: Wed, 22 Oct 2014 14:20:08 -0700 (PDT) X-CMAE-Score: 0 X-CMAE-Analysis: v=2.1 cv=SvUDtp+0 c=1 sm=1 tr=0 a=WkljmVdYkabdwxfqvArNOQ==:117 a=14OXPxybAAAA:8 a=Z6MIti7PxpgA:10 a=kj9zAlcOel0A:10 a=hOe2yjtxAAAA:8 a=GWvEu0HoXAjE_DOeDy0A:9 a=CjuIK1q_8ugA:10 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 * 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. --- 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 #include #include +#include #include #include @@ -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 #include #include +#include #include #include #include @@ -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))