[v2,16/23] nscd: Convert group client to __nscd_generic_get

  ---
 nscd/nscd_getgr_r.c | 250 ++++----------------------------------------
 1 file changed, 19 insertions(+), 231 deletions(-)
  

On 3/20/26 4:42 PM, Florian Weimer wrote:

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>   nscd/nscd_getgr_r.c | 250 ++++----------------------------------------
>   1 file changed, 19 insertions(+), 231 deletions(-)
> 
> diff --git a/nscd/nscd_getgr_r.c b/nscd/nscd_getgr_r.c
> index f77650b25f..daf046bcbf 100644
> --- a/nscd/nscd_getgr_r.c
> +++ b/nscd/nscd_getgr_r.c
> @@ -31,21 +31,21 @@
>   #include <not-cancel.h>
>   #include <_itoa.h>
>   #include <scratch_buffer.h>
> +#include <nss/nss_generic.h>
>   
>   #include "nscd-client.h"
>   #include "nscd-dbtype.h"
>   #include "nscd_proto.h"
>   
> -static int nscd_getgr_r (const char *key, size_t keylen, request_type type,
> -			 struct group *resultbuf, char *buffer,
> -			 size_t buflen, struct group **result);
> -
> +static int nscd_getgr_r (enum nss_lookup_type lt, const void *key,
> +			 struct group *resultbuf, char *buffer, size_t buflen,
> +			 struct group **result);
>   
>   int
>   __nscd_getgrnam_r (const char *name, struct group *resultbuf, char *buffer,
>   		   size_t buflen, struct group **result)
>   {
> -  return nscd_getgr_r (name, strlen (name) + 1, GETGRBYNAME, resultbuf,
> +  return nscd_getgr_r (nss_lookup_getgrnam, name, resultbuf,
>   		       buffer, buflen, result);
>   }
>   
> @@ -54,241 +54,29 @@ int
>   __nscd_getgrgid_r (gid_t gid, struct group *resultbuf, char *buffer,
>   		   size_t buflen, struct group **result)
>   {
> -  char buf[3 * sizeof (gid_t)];
> -  buf[sizeof (buf) - 1] = '\0';
> -  char *cp = _itoa_word (gid, buf + sizeof (buf) - 1, 10, 0);
> -
> -  return nscd_getgr_r (cp, buf + sizeof (buf) - cp, GETGRBYGID, resultbuf,
> +  return nscd_getgr_r (nss_lookup_getgrgid, &gid, resultbuf,
>   		       buffer, buflen, result);
>   }
>   
>   static int
> -nscd_getgr_r (const char *key, size_t keylen, request_type type,
> +nscd_getgr_r (enum nss_lookup_type lt, const void *key,
>   	      struct group *resultbuf, char *buffer, size_t buflen,
>   	      struct group **result)
>   {
> -  int gc_cycle;
> -  int nretries = 0;
> -  const uint32_t *len = NULL;
> -  struct scratch_buffer lenbuf;
> -  scratch_buffer_init (&lenbuf);
> -
> -  /* If the mapping is available, try to search there instead of
> -     communicating with the nscd.  */
> -  struct mapped_database *map = __nscd_get_map_ref (grpdb, &gc_cycle);
> - retry:;
> -  const char *gr_name = NULL;
> -  size_t gr_name_len = 0;
> -  int retval = -1;
> -  const char *recend = (const char *) ~UINTMAX_C (0);
> -  gr_response_header gr_resp;
> -
> -  if (map != NULL)
> +  void *result1;
> +  bool ok = __nscd_generic_get (lt, key, &result1);
> +  if (ok)
>       {
> -      struct datahead *found = __nscd_cache_search (type, key, keylen, map,
> -						    sizeof gr_resp);
> -      if (found != NULL)
> -	{
> -	  len = (const uint32_t *) (&found->data[0].grdata + 1);
> -	  gr_resp = found->data[0].grdata;
> -	  gr_name = ((const char *) len
> -		     + gr_resp.gr_mem_cnt * sizeof (uint32_t));
> -	  gr_name_len = gr_resp.gr_name_len + gr_resp.gr_passwd_len;
> -	  recend = (const char *) found->data + found->recsize;
> -	  /* Now check if we can trust gr_resp fields.  If GC is
> -	     in progress, it can contain anything.  */
> -	  if (map->head->gc_cycle != gc_cycle)
> -	    {
> -	      retval = -2;
> -	      goto out;
> -	    }
> -
> -	  /* The alignment is always sufficient, unless GC is in progress.  */
> -	  assert (((uintptr_t) len & (__alignof__ (*len) - 1)) == 0);
> -	}
> -    }
> -
> -  int sock = -1;
> -  if (gr_name == NULL)
> -    {
> -      sock = __nscd_open_socket (key, keylen, type, &gr_resp,
> -				 sizeof (gr_resp));
> -      if (sock == -1)
> -	{
> -	  __nscd_defer_database (grpdb);
> -	  goto out;
> -	}
> -    }
> -
> -  /* No value found so far.  */
> -  *result = NULL;
> -
> -  if (__glibc_unlikely (gr_resp.found == -1))
> -    {
> -      /* The daemon does not cache this database.  */
> -      __nscd_defer_database (grpdb);
> -      goto out_close;
> -    }
> -
> -  if (gr_resp.found == 1)
> -    {
> -      struct iovec vec[2];
> -      char *p = buffer;
> -      size_t total_len;
> -      uintptr_t align;
> -      nscd_ssize_t cnt;
> -
> -      /* Now allocate the buffer the array for the group members.  We must
> -	 align the pointer.  */
> -      align = ((__alignof__ (char *) - ((uintptr_t) p))
> -	       & (__alignof__ (char *) - 1));
> -      total_len = (align + (1 + gr_resp.gr_mem_cnt) * sizeof (char *)
> -		   + gr_resp.gr_name_len + gr_resp.gr_passwd_len);
> -      if (__glibc_unlikely (buflen < total_len))
> -	{
> -	no_room:
> -	  __set_errno (ERANGE);
> -	  retval = ERANGE;
> -	  goto out_close;
> -	}
> -      buflen -= total_len;
> -
> -      p += align;
> -      resultbuf->gr_mem = (char **) p;
> -      p += (1 + gr_resp.gr_mem_cnt) * sizeof (char *);
> -
> -      /* Set pointers for strings.  */
> -      resultbuf->gr_name = p;
> -      p += gr_resp.gr_name_len;
> -      resultbuf->gr_passwd = p;
> -      p += gr_resp.gr_passwd_len;
> -
> -      /* Fill in what we know now.  */
> -      resultbuf->gr_gid = gr_resp.gr_gid;
> -
> -      /* Read the length information, group name, and password.  */
> -      if (gr_name == NULL)
> -	{
> -	  /* Handle a simple, usual case: no group members.  */
> -	  if (__glibc_likely (gr_resp.gr_mem_cnt == 0))
> -	    {
> -	      size_t n = gr_resp.gr_name_len + gr_resp.gr_passwd_len;
> -	      if (__builtin_expect (__readall (sock, resultbuf->gr_name, n)
> -				    != (ssize_t) n, 0))
> -		goto out_close;
> -	    }
> -	  else
> -	    {
> -	      /* Allocate array to store lengths.  */
> -	      if (!scratch_buffer_set_array_size
> -		  (&lenbuf, gr_resp.gr_mem_cnt, sizeof (uint32_t)))
> -		goto out_close;
> -	      len = lenbuf.data;
> -
> -	      vec[0].iov_base = (void *) len;
> -	      vec[0].iov_len = gr_resp.gr_mem_cnt * sizeof (uint32_t);
> -	      vec[1].iov_base = resultbuf->gr_name;
> -	      vec[1].iov_len = gr_resp.gr_name_len + gr_resp.gr_passwd_len;
> -	      total_len = vec[0].iov_len + vec[1].iov_len;
> -
> -	      /* Get this data.  */
> -	      size_t n = __readvall (sock, vec, 2);
> -	      if (__glibc_unlikely (n != total_len))
> -		goto out_close;
> -	    }
> -	}
> -      else
> -	/* We already have the data.  Just copy the group name and
> -	   password.  */
> -	memcpy (resultbuf->gr_name, gr_name,
> -		gr_resp.gr_name_len + gr_resp.gr_passwd_len);
> -
> -      /* Clear the terminating entry.  */
> -      resultbuf->gr_mem[gr_resp.gr_mem_cnt] = NULL;
> -
> -      /* Prepare reading the group members.  */
> -      total_len = 0;
> -      for (cnt = 0; cnt < gr_resp.gr_mem_cnt; ++cnt)
> +      if (result1 == NULL)
>   	{
> -	  resultbuf->gr_mem[cnt] = p;
> -	  total_len += len[cnt];
> -	  p += len[cnt];
> +	  *result = NULL;
> +	  return 0;
>   	}
> -
> -      if (__glibc_unlikely (gr_name + gr_name_len + total_len > recend))
> -	{
> -	  /* len array might contain garbage during nscd GC cycle,
> -	     retry rather than fail in that case.  */
> -	  if (gr_name != NULL && map->head->gc_cycle != gc_cycle)
> -	    retval = -2;
> -	  goto out_close;
> -	}
> -      if (__glibc_unlikely (total_len > buflen))
> -	{
> -	  /* len array might contain garbage during nscd GC cycle,
> -	     retry rather than fail in that case.  */
> -	  if (gr_name != NULL && map->head->gc_cycle != gc_cycle)
> -	    {
> -	      retval = -2;
> -	      goto out_close;
> -	    }
> -	  else
> -	    goto no_room;
> -	}
> -
> -      retval = 0;
> -
> -      /* If there are no group members TOTAL_LEN is zero.  */
> -      if (gr_name == NULL)
> -	{
> -	  if (total_len > 0
> -	      && __builtin_expect (__readall (sock, resultbuf->gr_mem[0],
> -					      total_len) != total_len, 0))
> -	    {
> -	      /* The `errno' to some value != ERANGE.  */
> -	      __set_errno (ENOENT);
> -	      retval = ENOENT;
> -	    }
> -	  else
> -	    *result = resultbuf;
> -	}
> -      else
> -	{
> -	  /* Copy the group member names.  */
> -	  memcpy (resultbuf->gr_mem[0], gr_name + gr_name_len, total_len);
> -
> -	  /* Try to detect corrupt databases.  */
> -	  if (resultbuf->gr_name[gr_name_len - 1] != '\0'
> -	      || resultbuf->gr_passwd[gr_resp.gr_passwd_len - 1] != '\0'
> -	      || ({for (cnt = 0; cnt < gr_resp.gr_mem_cnt; ++cnt)
> -		    if (resultbuf->gr_mem[cnt][len[cnt] - 1] != '\0')
> -		      break;
> -		  cnt < gr_resp.gr_mem_cnt; }))
> -	    {
> -	      /* We cannot use the database.  */
> -	      retval = map != NULL && map->head->gc_cycle != gc_cycle ? -2 : -1;
> -	      goto out_close;
> -	    }
> -
> -	  *result = resultbuf;
> -	}
> -    }
> -  else
> -    {
> -      /* Set errno to 0 to indicate no error, just no found record.  */
> -      __set_errno (0);
> -      /* Even though we have not found anything, the result is zero.  */
> -      retval = 0;
> +      int ret = __nss_generic_copy (lt, result1, resultbuf, buffer, buflen);

It bears showing the beauty of the patch applied...

  61 static int
  62 nscd_getgr_r (enum nss_lookup_type lt, const void *key,
  63               struct group *resultbuf, char *buffer, size_t buflen,
  64               struct group **result)
  65 {
  66   void *result1;
  67   bool ok = __nscd_generic_get (lt, key, &result1);
  68   if (ok)
  69     {
  70       if (result1 == NULL)
  71         {
  72           *result = NULL;
  73           return 0;
  74         }
  75       int ret = __nss_generic_copy (lt, result1, resultbuf, buffer, buflen);
  76       free (result1);
  77       if (ret == 0)
  78         *result = resultbuf;
  79       return ret;
  80     }
  81   return -1;
  82 }


OK. Nice :-)

> +      free (result1);
> +      if (ret == 0)
> +	*result = resultbuf;
> +      return ret;
>       }
> -
> - out_close:
> -  if (sock != -1)
> -    __close_nocancel_nostatus (sock);
> - out:
> -  if (__nscd_map_ref_retry_or_drop (&map, &gc_cycle, &nretries, retval))
> -    goto retry;
> -
> -  scratch_buffer_free (&lenbuf);
> -
> -  return retval;
> +  return -1;
>   }