From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-3.9 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id B23221F66F for ; Tue, 3 Nov 2020 16:56:13 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 335D539BA420; Tue, 3 Nov 2020 16:56:12 +0000 (GMT) Received: from hedgehog.birch.relay.mailchannels.net (hedgehog.birch.relay.mailchannels.net [23.83.209.81]) by sourceware.org (Postfix) with ESMTPS id 16B693986804 for ; Tue, 3 Nov 2020 16:56:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 16B693986804 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gotplt.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=siddhesh@gotplt.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id A5D557002D1; Tue, 3 Nov 2020 16:56:06 +0000 (UTC) Received: from pdx1-sub0-mail-a31.g.dreamhost.com (100-100-138-5.trex.outbound.svc.cluster.local [100.100.138.5]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 345D8700059; Tue, 3 Nov 2020 16:56:06 +0000 (UTC) X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from pdx1-sub0-mail-a31.g.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384) by 0.0.0.0:2500 (trex/5.18.10); Tue, 03 Nov 2020 16:56:06 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Trouble-Occur: 2bf29c48508e65ab_1604422566524_1728085242 X-MC-Loop-Signature: 1604422566524:4250078936 X-MC-Ingress-Time: 1604422566524 Received: from pdx1-sub0-mail-a31.g.dreamhost.com (localhost [127.0.0.1]) by pdx1-sub0-mail-a31.g.dreamhost.com (Postfix) with ESMTP id B20F67F0DF; Tue, 3 Nov 2020 08:56:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gotplt.org; h=subject:to :references:from:message-id:date:mime-version:in-reply-to :content-type:content-transfer-encoding; s=gotplt.org; bh=bfFfVM gEtnuMy2R2al4zvXxMn9Y=; b=QTgyQgqRsIKt4lwQLcx/kT1sIwgy5FSTSNx4iy trj4Ugvwhtn9+cSj9pmx5AVmgLzl4qD4cewUNGxlFZr7HRCc8hgOKxmJoWvBuWwD bHzCoIRJmgQMTTkiKr33ac+XQoUwjS4TP5ClP4KWtxn/XCKBgkYXC6vpr9MKxN0J CkE3s= Received: from [192.168.1.111] (unknown [123.252.202.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a31.g.dreamhost.com (Postfix) with ESMTPSA id 183707F130; Tue, 3 Nov 2020 08:56:03 -0800 (PST) Subject: Re: v3 [PATCH 2/4] : New abstraction for combining NSS modules To: DJ Delorie , libc-alpha@sourceware.org References: X-DH-BACKEND: pdx1-sub0-mail-a31 From: Siddhesh Poyarekar Message-ID: Date: Tue, 3 Nov 2020 22:25:40 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" On 8/28/20 12:11 AM, DJ Delorie via Libc-alpha wrote: > > From 72fcac82c3ed80a168f0f31eaf8063208a44ff32 Mon Sep 17 00:00:00 2001 > From: Florian Weimer > Date: Thu, 20 Feb 2020 09:32:27 +0100 > Subject: [PATCH 2/4] : New abstraction for combining NSS modules > and NSS actions > > --- > nss/Makefile | 3 +- > nss/nss_action.c | 116 ++++++++++++++++++++++++ > nss/nss_action.h | 106 ++++++++++++++++++++++ > nss/nss_action_parse.c | 197 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 421 insertions(+), 1 deletion(-) > create mode 100644 nss/nss_action.c > create mode 100644 nss/nss_action.h > create mode 100644 nss/nss_action_parse.c > > diff --git a/nss/Makefile b/nss/Makefile > index 18035a4fb4..f3e555066b 100644 > --- a/nss/Makefile > +++ b/nss/Makefile > @@ -30,7 +30,8 @@ routines = nsswitch getnssent getnssent_r digits_dots \ > $(addsuffix -lookup,$(databases)) \ > compat-lookup nss_hash nss_files_fopen \ > nss_readline nss_parse_line_result \ > - nss_fgetent_r nss_module > + nss_fgetent_r nss_module nss_action \ > + nss_action_parse > > # 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/nss_action.c b/nss/nss_action.c > new file mode 100644 > index 0000000000..b9c3b93fea > --- /dev/null > +++ b/nss/nss_action.c > @@ -0,0 +1,116 @@ > +/* NSS actions, elements in a nsswitch.conf configuration line. > + Copyright (c) 2020 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 > + . */ > + > +#include > + > +#include > +#include > + > +/* Maintain a global list of NSS action lists. Since most databases > + use the same list of actions, this list is usually short. > + Deduplication in __nss_action_allocate ensures that the list does > + not grow without bounds. */ > + > +struct nss_action_list_wrapper > +{ > + /* The next element of the list. */ > + struct nss_action_list_wrapper *next; > + > + /* Number of elements in the list (excluding the terminator). */ > + size_t count; > + > + /* NULL-terminated list of actions. */ > + struct nss_action actions[]; > +}; > + > +/* Toplevel list of allocated NSS action lists. */ > +static struct nss_action_list_wrapper *nss_actions; > + > +/* Lock covers the nss_actions list. */ > +__libc_lock_define (static, nss_actions_lock); > + > +/* Returns true if the actions are equal (same module, same actions > + array). */ > +static bool > +actions_equal (const struct nss_action *a, const struct nss_action *b) > +{ > + return a->module == b->module && a->action_bits == b->action_bits; > +} > + > + > +/* Returns true if COUNT actions at A and B are equal (according to > + actions_equal above). Caller must ensure that either A or B have at > + least COUNT actions. */ > +static bool > +action_lists_equal (const struct nss_action *a, const struct nss_action *b, > + size_t count) > +{ > + for (size_t i = 0; i < count; ++i) > + if (!actions_equal (a + i, b + i)) > + return false; > + return true; > +} > + > +/* Returns a pre-allocated action list for COUNT actions at ACTIONS, > + or NULL if no such list exists. */ > +static nss_action_list > +find_allocated (struct nss_action *actions, size_t count) > +{ > + for (struct nss_action_list_wrapper *p = nss_actions; p != NULL; p = p->next) > + if (p->count == count && action_lists_equal (p->actions, actions, count)) > + return p->actions; > + return NULL; > +} > + > +nss_action_list > +__nss_action_allocate (struct nss_action *actions, size_t count) > +{ > + nss_action_list result = NULL; > + __libc_lock_lock (nss_actions_lock); > + > + result = find_allocated (actions, count); > + if (result == NULL) > + { > + struct nss_action_list_wrapper *wrapper > + = malloc (sizeof (*wrapper) + sizeof (*actions) * count); > + if (wrapper != NULL) > + { > + wrapper->next = nss_actions; > + wrapper->count = count; > + memcpy (wrapper->actions, actions, sizeof (*actions) * count); > + nss_actions = wrapper; > + result = wrapper->actions; > + } > + } > + > + __libc_lock_unlock (nss_actions_lock); > + return result; > +} > + > +void __libc_freeres_fn_section > +__nss_action_freeres (void) > +{ > + struct nss_action_list_wrapper *current = nss_actions; > + while (current != NULL) > + { > + struct nss_action_list_wrapper *next = current->next; > + free (current); > + current = next; > + } > + nss_actions = NULL; > +} > diff --git a/nss/nss_action.h b/nss/nss_action.h > new file mode 100644 > index 0000000000..bd93c6e67c > --- /dev/null > +++ b/nss/nss_action.h > @@ -0,0 +1,106 @@ > +/* NSS actions, elements in a nsswitch.conf configuration line. > + Copyright (c) 2020 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 > + . */ > + > +#ifndef _NSS_ACTION_H > +#define _NSS_ACTION_H > + > +#include > + > +#include "nsswitch.h" /* For lookup_actions. */ > + > +struct nss_module; > + > +/* A NSS action pairs a service module with the action for each result > + state. */ > +struct nss_action > +{ > + /* The service module that provides the functionality (potentially > + not yet loaded). */ > + struct nss_module *module; > + > + /* Action according to result. Two bits for each lookup_actions > + value (from nsswitch.h), indexed by enum nss_status (from nss.h). */ > + unsigned int action_bits; > +}; > + > +/* Value to add to first nss_status value to get zero. */ > +#define NSS_STATUS_BIAS 2 > +/* Number of bits per lookup action. */ > +#define NSS_BPL 2 > +#define NSS_BPL_MASK ((1 << NSS_BPL) - 1) > + > +/* Index in actions of an NSS status. Note that in nss/nss.h the > + status starts at -2, and we shift that up to zero by adding 2. > + Thus for example NSS_STATUS_TRYAGAIN, which is -2, would index into > + the 0th bit place as expected. */ > +static inline int > +nss_actions_bits_index (enum nss_status status) > +{ > + return NSS_BPL * (NSS_STATUS_BIAS + status); > +} > + > +/* Returns the lookup_action value for STATUS in ACTION. */ > +static inline lookup_actions > +nss_action_get (const struct nss_action *action, enum nss_status status) > +{ > + return ((action->action_bits >> nss_actions_bits_index (status)) > + & NSS_BPL_MASK); > +} > + > +/* Sets the lookup_action value for STATUS in ACTION. */ > +static inline void > +nss_action_set (struct nss_action *action, > + enum nss_status status, lookup_actions actions) > +{ > + int offset = nss_actions_bits_index (status); > + unsigned int mask = NSS_BPL_MASK << offset; > + action->action_bits = ((action->action_bits & ~mask) > + | ((unsigned int) actions << offset)); > +} > + > +static inline void > +nss_action_set_all (struct nss_action *action, lookup_actions actions) > +{ > + unsigned int bits = actions & NSS_BPL_MASK; > + action->action_bits = ( bits > + | (bits << (NSS_BPL * 1)) > + | (bits << (NSS_BPL * 2)) > + | (bits << (NSS_BPL * 3)) > + | (bits << (NSS_BPL * 4)) > + ); > +} > + > +/* A list of struct nss_action objects in array terminated by an > + action with a NULL module. */ > +typedef struct nss_action *nss_action_list; > + > +/* Returns a pointer to an allocated NSS action list that has COUNT > + actions that matches the array at ACTIONS. */ > +nss_action_list __nss_action_allocate (struct nss_action *actions, > + size_t count) attribute_hidden; > + > +/* Returns a pointer to a list allocated by __nss_action_allocate, or > + NULL on error. ENOMEM means a (temporary) memory allocation error, > + EINVAL means that LINE is syntactically invalid. */ > +nss_action_list __nss_action_parse (const char *line); > + > +/* Called from __libc_freeres. */ > +void __nss_action_freeres (void) attribute_hidden; > + > + > +#endif /* _NSS_ACTION_H */ > diff --git a/nss/nss_action_parse.c b/nss/nss_action_parse.c > new file mode 100644 > index 0000000000..ada3acbb08 > --- /dev/null > +++ b/nss/nss_action_parse.c > @@ -0,0 +1,197 @@ > +/* Parse a service line from nsswitch.conf. > + Copyright (c) 1996-2020 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 > + . */ > + > +#include "nss_action.h" > +#include "nss_module.h" > + > +#include > +#include > +#include > + > +/* Staging area during parsing. */ > +#define DYNARRAY_STRUCT action_list > +#define DYNARRAY_ELEMENT struct nss_action > +#define DYNARRAY_PREFIX action_list_ > +#include > + > + > +/* Read the source names: > + `( ( "[" "!"? ( "=" )+ "]" )? )*' > + */ > +static bool > +nss_action_parse (const char *line, struct action_list *result) > +{ > + while (1) > + { > + while (isspace (line[0])) > + ++line; > + if (line[0] == '\0') > + /* No source specified. */ > + return result; We found no source; maybe you meant 'return true' here? > + > + /* Read identifier. */ > + const char *name = line; > + while (line[0] != '\0' && !isspace (line[0]) && line[0] != '[') > + ++line; > + if (name == line) > + return true; We found a name and no actions. OK. > + > + struct nss_action new_service > + = { .module = __nss_module_allocate (name, line - name), }; > + if (new_service.module == NULL) > + { > + /* Memory allocation error. */ > + action_list_mark_failed (result); > + return false; > + } > + nss_action_set_all (&new_service, NSS_ACTION_CONTINUE); > + nss_action_set (&new_service, NSS_STATUS_SUCCESS, NSS_ACTION_RETURN); > + nss_action_set (&new_service, NSS_STATUS_RETURN, NSS_ACTION_RETURN); Default actions. OK. > + > + while (isspace (line[0])) > + ++line; > + > + if (line[0] == '[') > + { > + /* Read criterions. */ > + do > + ++line; > + while (line[0] != '\0' && isspace (line[0])); > + > + do > + { > + int not; > + enum nss_status status; > + lookup_actions action; > + > + /* Grok ! before name to mean all statii but that one. */ s/statii/statuses/ > + not = line[0] == '!'; > + if (not) > + ++line; > + > + /* Read status name. */ > + name = line; > + while (line[0] != '\0' && !isspace (line[0]) && line[0] != '=' > + && line[0] != ']') > + ++line; > + > + /* Compare with known statii. */ s/statii/statuses/ > + if (line - name == 7) > + { > + if (__strncasecmp (name, "SUCCESS", 7) == 0) > + status = NSS_STATUS_SUCCESS; > + else if (__strncasecmp (name, "UNAVAIL", 7) == 0) > + status = NSS_STATUS_UNAVAIL; > + else > + return false; > + } > + else if (line - name == 8) > + { > + if (__strncasecmp (name, "NOTFOUND", 8) == 0) > + status = NSS_STATUS_NOTFOUND; > + else if (__strncasecmp (name, "TRYAGAIN", 8) == 0) > + status = NSS_STATUS_TRYAGAIN; > + else > + return false; > + } > + else > + return false; OK. > + > + while (isspace (line[0])) > + ++line; Looks like the function might benefit from a skip_over() macro since this idiom is repeating all over the place with some minor variations. It's not a strong suggestion though, so you may ignore it if you like this better. > + if (line[0] != '=') > + return false; > + do > + ++line; > + while (isspace (line[0])); > + > + name = line; > + while (line[0] != '\0' && !isspace (line[0]) && line[0] != '=' > + && line[0] != ']') > + ++line; > + > + if (line - name == 6 && __strncasecmp (name, "RETURN", 6) == 0) > + action = NSS_ACTION_RETURN; > + else if (line - name == 8 > + && __strncasecmp (name, "CONTINUE", 8) == 0) > + action = NSS_ACTION_CONTINUE; > + else if (line - name == 5 > + && __strncasecmp (name, "MERGE", 5) == 0) > + action = NSS_ACTION_MERGE; > + else > + return false; > + > + if (not) > + { > + /* Save the current action setting for this status, > + set them all to the given action, and reset this one. */ > + const lookup_actions save > + = nss_action_get (&new_service, status); > + nss_action_set_all (&new_service, action); > + nss_action_set (&new_service, status, save); > + } > + else > + nss_action_set (&new_service, status, action); > + > + /* Skip white spaces. */ > + while (isspace (line[0])) > + ++line; > + } > + while (line[0] != ']'); > + > + /* Skip the ']'. */ > + ++line; > + } > + > + action_list_add (result, new_service); > + } > +} > + > +nss_action_list > + __nss_action_parse (const char *line) > +{ > + struct action_list list; > + action_list_init (&list); > + if (nss_action_parse (line, &list)) > + { > + size_t size = action_list_size (&list); > + nss_action_list result > + = malloc (sizeof (*result) * (size + 1)); > + if (result == NULL) > + { > + action_list_free (&list); > + return NULL; > + } > + memcpy (result, action_list_begin (&list), sizeof (*result) * size); > + /* Sentinel. */ > + result[size].module = NULL; > + return result; > + } > + else if (action_list_has_failed (&list)) > + { > + /* Memory allocation error. */ > + __set_errno (ENOMEM); > + return NULL; > + } > + else > + { > + /* Parse error. */ > + __set_errno (EINVAL); > + return NULL; > + } > +} > OK. Siddhesh