[PATCH] nis+/getgrent: don't set errno when there are no more entries

[Jim Meyering <jim at meyering dot net>]

There is a bug in group-related nis+ support.
[FYI, I've just filed this as http://bugzilla.redhat.com/437945, too]

First symptom was this report against id from coreutils':

    http://savannah.gnu.org/bugs/?22505

Here's code to demonstrate the bug:

cat <<\EOF > getgrent-test.c
#include <sys/types.h>
#include <grp.h>
#include <errno.h>
#include <stdio.h>

static int
count_groups (void)
{
  int count = 0;

  setgrent ();
  while (1)
    {
      errno = 0;
      struct group *grp = getgrent ();
      if (!grp)
        break;
      ++count;
    }

  if (errno != 0)
    {
      perror ("getgrent failed");
      count = -1;
    }

  int saved_errno = errno;
  endgrent ();
  errno = saved_errno;

  return count;
}

int
main ()
{
  return count_groups () < 0;
}
EOF
gcc -O -W -Wall getgrent-test.c && ./a.out && echo ok
======================================

When I run that on a rawhide system on which /etc/nsswitch.conf has this:

  group: files

I get an exit status of 0, as expected.
However, when I change nsswitch.conf, appending " nisplus"
(and I haven't configured NIS+ at all):

  group: files nisplus

and run ./a.out again, I get this output:

  getgrent failed: No such file or directory
  [Exit 1]

Investigating, I saw suspicious differences between the
_nss_nisplus_getgrgid_r and _nss_nisplus_getgrnam_r
functions in nis/nss_nisplus/nisplus-grp.c.  They should be nearly
identical.  Eliminating one difference fixes the bug.
Eliminating the other avoids a useless invocation of __set_errno.
Here's the patch, which also normalizes some inconsequential parts,
too, so that the difference between the two functions is minimal.

I've confirmed this also affects at least RHEL4.6 and FC5.

2008-03-18  Jim Meyering  <meyering@redhat.com>

	nis+/getgrent: don't set errno when there are no more entries
	* nis/nss_nisplus/nisplus-grp.c (_nss_nisplus_getgrnam_r): Restore
	saved errno value upon EOF.
	(_nss_nisplus_getgrgid_r): Avoid useless __set_errno.

diff --git a/nis/nss_nisplus/nisplus-grp.c b/nis/nss_nisplus/nisplus-grp.c
index 7cc762f..5da5541 100644
--- a/nis/nss_nisplus/nisplus-grp.c
+++ b/nis/nss_nisplus/nisplus-grp.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 1997, 2001, 2002, 2003, 2005, 2006
+/* Copyright (C) 1997, 2001, 2002, 2003, 2005, 2006, 2008
    Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Thorsten Kukuk <kukuk@vt.uni-paderborn.de>, 1997.
@@ -288,8 +288,6 @@ enum nss_status
 _nss_nisplus_getgrnam_r (const char *name, struct group *gr,
 			 char *buffer, size_t buflen, int *errnop)
 {
-  int parse_res;
-
   if (grp_tablename_val == NULL)
     {
       enum nss_status status = _nss_grp_create_tablename (errnop);
@@ -304,6 +302,7 @@ _nss_nisplus_getgrnam_r (const char *name, struct group *gr,
       return NSS_STATUS_NOTFOUND;
     }

+  int parse_res;
   nis_result *result;
   char buf[strlen (name) + 9 + grp_tablename_len];
   int olderr = errno;
@@ -322,6 +321,8 @@ _nss_nisplus_getgrnam_r (const char *name, struct group *gr,
     {
       enum nss_status status = niserr2nss (result->status);

+      __set_errno (olderr);
+
       nis_freeresult (result);
       return status;
     }
@@ -365,7 +366,7 @@ _nss_nisplus_getgrgid_r (const gid_t gid, struct group *gr,
   snprintf (buf, sizeof (buf), "[gid=%lu],%s",
 	    (unsigned long int) gid, grp_tablename_val);

-  result = nis_list (buf, FOLLOW_PATH | FOLLOW_LINKS, NULL, NULL);
+  result = nis_list (buf, FOLLOW_LINKS | FOLLOW_PATH, NULL, NULL);

   if (result == NULL)
     {
@@ -384,19 +385,19 @@ _nss_nisplus_getgrgid_r (const gid_t gid, struct group *gr,
     }

   parse_res = _nss_nisplus_parse_grent (result, gr, buffer, buflen, errnop);
-
   nis_freeresult (result);
   if (__builtin_expect (parse_res < 1, 0))
     {
-      __set_errno (olderr);
-
       if (parse_res == -1)
 	{
 	  *errnop = ERANGE;
 	  return NSS_STATUS_TRYAGAIN;
 	}
       else
-	return NSS_STATUS_NOTFOUND;
+	{
+	  __set_errno (olderr);
+	  return NSS_STATUS_NOTFOUND;
+	}
     }

   return NSS_STATUS_SUCCESS;
--
1.5.5.rc0.7.g57e83