[PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[Steve Ellcey]

This patch is based on the discussion string at:

https://sourceware.org/ml/libc-alpha/2017-08/msg00531.html

Basically a change was made to remove the nss compat lookup
method by default from glibc.  It is available if glibc
is configured with --enable-obsolete-nsl.  Some tests were
still using the compat lookup method because DEFAULT_CONFIG
still included that method in the files below.  Depending on
how you test that might not cause any failures because if
libnss_compat could not be found in the glibc build directories
the dynamic linker would search the default host directories
and pick it up from there, only if it couldn't find it in
either place would the tests (grp/testgrp, nss/bug17079,
posix/globtest, and pwd/tst-getpwfail) fail.

This patch removes the compat lookup method from the
DEFAULT_CONFIG strings.  Tested on x86 and aarch64 with
no regressions.

OK to checkin?


2017-08-17  Steve Ellcey  <sellcey@cavium.com>

	* nss/grp-lookup.c (DEFAULT_CONFIG): Remove compat method.
	* nss/pwd-lookup.c (DEFAULT_CONFIG): Likewise.
	* nss/spwd-lookup.c (DEFAULT_CONFIG): Likewise.


diff --git a/nss/grp-lookup.c b/nss/grp-lookup.c
index 8cb00aa..256d2af 100644
--- a/nss/grp-lookup.c
+++ b/nss/grp-lookup.c
@@ -17,6 +17,6 @@
    <http://www.gnu.org/licenses/>.  */
 
 #define DATABASE_NAME group
-#define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#define DEFAULT_CONFIG "files"
 
 #include "XXX-lookup.c"
diff --git a/nss/pwd-lookup.c b/nss/pwd-lookup.c
index 00040d4..8612c39 100644
--- a/nss/pwd-lookup.c
+++ b/nss/pwd-lookup.c
@@ -17,6 +17,6 @@
    <http://www.gnu.org/licenses/>.  */
 
 #define DATABASE_NAME passwd
-#define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#define DEFAULT_CONFIG "files"
 
 #include "XXX-lookup.c"
diff --git a/nss/spwd-lookup.c b/nss/spwd-lookup.c
index 319a7bb..2c866d1 100644
--- a/nss/spwd-lookup.c
+++ b/nss/spwd-lookup.c
@@ -18,6 +18,6 @@
 
 #define DATABASE_NAME shadow
 #define ALTERNATE_NAME passwd
-#define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#define DEFAULT_CONFIG "files"
 
 #include "XXX-lookup.c"

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[DJ Delorie]

Also found (crude search) in:

./nss/nsswitch.c:        nss_load_all_libraries ("passwd", "compat [NOTFOUND=return] files"); 
./nss/nsswitch.c:        nss_load_all_libraries ("group", "compat [NOTFOUND=return] files"); 
./grp/initgroups.c:      no_more = __nss_database_lookup ("group", NULL, "compat files", 
./nscd/initgrcache.c:    "compat [NOTFOUND=return] files", 

Do these need to be tweaked also?

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[Steve Ellcey]

On Thu, 2017-08-17 at 20:18 -0400, DJ Delorie wrote:
> Also found (crude search) in:
> 
> ./nss/nsswitch.c:        nss_load_all_libraries ("passwd", "compat
> [NOTFOUND=return] files"); 
> ./nss/nsswitch.c:        nss_load_all_libraries ("group", "compat
> [NOTFOUND=return] files"); 
> ./grp/initgroups.c:      no_more = __nss_database_lookup ("group",
> NULL, "compat files", 
> ./nscd/initgrcache.c:    "compat [NOTFOUND=return] files", 
> 
> Do these need to be tweaked also?

I would think so.  I did a search and didn't find any more "compat"
lookups beyond what you found.  Not doing these changes didn't cause
any testsuite failures but it does seem like they should be changed.  I
updated them and reran the testsuite and got no regressions.  Here is
an updated patch.

Steve Ellcey
sellcey@cavium.com


2017-08-18  Steve Ellcey  <sellcey@cavium.com>

	* nss/grp-lookup.c (DEFAULT_CONFIG): Remove compat method.
	* nss/pwd-lookup.c (DEFAULT_CONFIG): Likewise.
	* nss/spwd-lookup.c (DEFAULT_CONFIG): Likewise.
	* grp/initgroups.c (internal_getgrouplist): Update lookup
	methods in __nss_database_lookup call.
	* nscd/initgrcache.c (addinitgroupsX): Likewise.
	* nss/nsswitch.c (__nss_disable_nscd): Likewise.


diff --git a/grp/initgroups.c b/grp/initgroups.c
index 0d5b841..9aef82c 100644
--- a/grp/initgroups.c
+++ b/grp/initgroups.c
@@ -79,7 +79,7 @@ internal_getgrouplist (const char *user, gid_t group, long int *size,
 				 &__nss_initgroups_database) < 0)
 	{
 	  if (__nss_group_database == NULL)
-	    no_more = __nss_database_lookup ("group", NULL, "compat files",
+	    no_more = __nss_database_lookup ("group", NULL, "files",
 					     &__nss_group_database);
 
 	  __nss_initgroups_database = __nss_group_database;
diff --git a/nscd/initgrcache.c b/nscd/initgrcache.c
index 4deb483..71a5f6c 100644
--- a/nscd/initgrcache.c
+++ b/nscd/initgrcache.c
@@ -85,9 +85,7 @@ addinitgroupsX (struct database_dyn *db, int fd, request_header *req,
   int no_more;
 
   if (group_database == NULL)
-    no_more = __nss_database_lookup ("group", NULL,
-				     "compat [NOTFOUND=return] files",
-				     &group_database);
+    no_more = __nss_database_lookup ("group", NULL, "files", &group_database);
   else
     no_more = 0;
   nip = group_database;
diff --git a/nss/grp-lookup.c b/nss/grp-lookup.c
index 8cb00aa..256d2af 100644
--- a/nss/grp-lookup.c
+++ b/nss/grp-lookup.c
@@ -17,6 +17,6 @@
    <http://www.gnu.org/licenses/>.  */
 
 #define DATABASE_NAME group
-#define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#define DEFAULT_CONFIG "files"
 
 #include "XXX-lookup.c"
diff --git a/nss/nsswitch.c b/nss/nsswitch.c
index 8f31658..349cd1c 100644
--- a/nss/nsswitch.c
+++ b/nss/nsswitch.c
@@ -848,8 +848,8 @@ __nss_disable_nscd (void (*cb) (size_t, struct traced_file *))
   is_nscd = true;
 
   /* Find all the relevant modules so that the init functions are called.  */
-  nss_load_all_libraries ("passwd", "compat [NOTFOUND=return] files");
-  nss_load_all_libraries ("group", "compat [NOTFOUND=return] files");
+  nss_load_all_libraries ("passwd", "files");
+  nss_load_all_libraries ("group", "files");
   nss_load_all_libraries ("hosts", "dns [!UNAVAIL=return] files");
   nss_load_all_libraries ("services", NULL);
 
diff --git a/nss/pwd-lookup.c b/nss/pwd-lookup.c
index 00040d4..8612c39 100644
--- a/nss/pwd-lookup.c
+++ b/nss/pwd-lookup.c
@@ -17,6 +17,6 @@
    <http://www.gnu.org/licenses/>.  */
 
 #define DATABASE_NAME passwd
-#define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#define DEFAULT_CONFIG "files"
 
 #include "XXX-lookup.c"
diff --git a/nss/spwd-lookup.c b/nss/spwd-lookup.c
index 319a7bb..2c866d1 100644
--- a/nss/spwd-lookup.c
+++ b/nss/spwd-lookup.c
@@ -18,6 +18,6 @@
 
 #define DATABASE_NAME shadow
 #define ALTERNATE_NAME passwd
-#define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#define DEFAULT_CONFIG "files"
 
 #include "XXX-lookup.c"

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[Joseph Myers]

[-- Attachment #1: Type: text/plain, Size: 594 bytes --]

On Fri, 18 Aug 2017, Steve Ellcey wrote:

> I would think so.  I did a search and didn't find any more "compat"
> lookups beyond what you found.  Not doing these changes didn't cause
> any testsuite failures but it does seem like they should be changed.  I
> updated them and reran the testsuite and got no regressions.  Here is
> an updated patch.

Does this affect any defaults visible to glibc users?  If it does, it 
needs an entry in the "Deprecated and removed features, and other changes 
affecting compatibility" section of NEWS.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[Steve Ellcey]

On Fri, 2017-08-18 at 21:12 +0000, Joseph Myers wrote:
> On Fri, 18 Aug 2017, Steve Ellcey wrote:
> 
> > 
> > I would think so.  I did a search and didn't find any more "compat"
> > lookups beyond what you found.  Not doing these changes didn't cause
> > any testsuite failures but it does seem like they should be changed.  I
> > updated them and reran the testsuite and got no regressions.  Here is
> > an updated patch.
> Does this affect any defaults visible to glibc users?  If it does, it 
> needs an entry in the "Deprecated and removed features, and other changes 
> affecting compatibility" section of NEWS.

I think everything is already in the 2.26 Depreciated section.  This
was done when the original patch to remove libnss_compat was made.
These changes are just dealing with the fact that that patch was
incomplete.

https://sourceware.org/ml/libc-alpha/2017-08/msg00010.html


* The NIS(+) name service modules, libnss_nis, libnss_nisplus, and
  libnss_compat, are deprecated, and will not be built or installed by
  default.

  The NIS(+) support library, libnsl, is also deprecated.  By default, a
  compatibility shared library will be built and installed, but not headers
  or development libraries. Only a few NIS-related programs require this
  library.  (In particular, the GNU C Library has never required programs
  that use 'gethostbyname' to be linked with libnsl.)

  Replacement implementations based on TIRPC, which additionally support
  IPv6, are available from <https://github.com/thkukuk/>.  The configure
  option --enable-obsolete-nsl will cause libnsl's headers, and the NIS(+)
  name service modules, to be built and installed.

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[Joseph Myers]

[-- Attachment #1: Type: text/plain, Size: 1173 bytes --]

On Fri, 18 Aug 2017, Steve Ellcey wrote:

> On Fri, 2017-08-18 at 21:12 +0000, Joseph Myers wrote:
> > On Fri, 18 Aug 2017, Steve Ellcey wrote:
> > 
> > > 
> > > I would think so.  I did a search and didn't find any more "compat"
> > > lookups beyond what you found.  Not doing these changes didn't cause
> > > any testsuite failures but it does seem like they should be changed.  I
> > > updated them and reran the testsuite and got no regressions.  Here is
> > > an updated patch.
> > Does this affect any defaults visible to glibc users?  If it does, it 
> > needs an entry in the "Deprecated and removed features, and other changes 
> > affecting compatibility" section of NEWS.
> 
> I think everything is already in the 2.26 Depreciated section.  This
> was done when the original patch to remove libnss_compat was made.
> These changes are just dealing with the fact that that patch was
> incomplete.

But the impression that NEWS entry gives is that you can use 
--enable-obsolete-nsl to get back the previous semantics.  Does your patch 
change the semantics in the --enable-obsolete-nsl case?

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[Steve Ellcey]

On Fri, 2017-08-18 at 22:12 +0000, Joseph Myers wrote:
> 
> But the impression that NEWS entry gives is that you can use 
> --enable-obsolete-nsl to get back the previous semantics.  Does your patch 
> change the semantics in the --enable-obsolete-nsl case?

That is a good point.  I hadn't considered it, but this patch does
change the semantics when --enable-obsolete-nsl is used.  I  guess the
question is: do we think that is OK and just need to document the
changes or do we need to ifdef the code changes so that when --enable-
obsolete-nsl is used, we get the existing behavour.  I am inclined to 
ifdef the code to preserve the current behavour under the enable
option.

Steve Ellcey
sellcey@cavium.com

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[Steve Ellcey]

[-- Attachment #1: Type: text/plain, Size: 904 bytes --]

On Fri, 2017-08-18 at 22:12 +0000, Joseph Myers wrote:
> 
> But the impression that NEWS entry gives is that you can use 
> --enable-obsolete-nsl to get back the previous semantics.  Does your patch 
> change the semantics in the --enable-obsolete-nsl case?


Here is an updated patch that maintains the original semantics and
default settings when --enable-obsolete-nsl is used.

Steve Ellcey
sellcey@cavium.com


2017-08-22  Steve Ellcey  <sellcey@cavium.com>

	* grp/initgroups.c: Include config.h.
	(DEFAULT_CONFIG): New macro.
	(internal_getgrouplist): Use DEFAULT_CONFIG.
	* nscd/initgrcache.c (addinitgroupsX): Likewise.
	* nss/nsswitch.c (__nss_disable_nscd): Likewise.

	* nss/grp-lookup.c: Include config.h
	(DEFAULT_CONFIG): Set definition based on LINK_OBSOLETE_NSL.
	* nss/pwd-lookup.c (DEFAULT_CONFIG): Likewise.
	* nss/spwd-lookup.c (DEFAULT_CONFIG): Likewise.


[-- Attachment #2: nls.compat.patch --]
[-- Type: text/x-patch, Size: 4484 bytes --]

diff --git a/grp/initgroups.c b/grp/initgroups.c
index 0d5b841..4448afe 100644
--- a/grp/initgroups.c
+++ b/grp/initgroups.c
@@ -26,10 +26,16 @@
 #include <sys/types.h>
 #include <nsswitch.h>
 #include <scratch_buffer.h>
+#include <config.h>
 
 #include "../nscd/nscd-client.h"
 #include "../nscd/nscd_proto.h"
 
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
 
 /* Type of the lookup function.  */
 typedef enum nss_status (*initgroups_dyn_function) (const char *, gid_t,
@@ -79,7 +85,7 @@ internal_getgrouplist (const char *user, gid_t group, long int *size,
 				 &__nss_initgroups_database) < 0)
 	{
 	  if (__nss_group_database == NULL)
-	    no_more = __nss_database_lookup ("group", NULL, "compat files",
+	    no_more = __nss_database_lookup ("group", NULL, DEFAULT_CONFIG,
 					     &__nss_group_database);
 
 	  __nss_initgroups_database = __nss_group_database;
diff --git a/nscd/initgrcache.c b/nscd/initgrcache.c
index 4deb483..4114fbe 100644
--- a/nscd/initgrcache.c
+++ b/nscd/initgrcache.c
@@ -25,6 +25,7 @@
 #include <unistd.h>
 #include <sys/mman.h>
 #include <scratch_buffer.h>
+#include <config.h>
 
 #include "dbg_log.h"
 #include "nscd.h"
@@ -34,6 +35,11 @@
 
 #include "../nss/nsswitch.h"
 
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
 
 /* Type of the lookup function.  */
 typedef enum nss_status (*initgroups_dyn_function) (const char *, gid_t,
@@ -85,8 +91,7 @@ addinitgroupsX (struct database_dyn *db, int fd, request_header *req,
   int no_more;
 
   if (group_database == NULL)
-    no_more = __nss_database_lookup ("group", NULL,
-				     "compat [NOTFOUND=return] files",
+    no_more = __nss_database_lookup ("group", NULL, DEFAULT_CONFIG,
 				     &group_database);
   else
     no_more = 0;
diff --git a/nss/grp-lookup.c b/nss/grp-lookup.c
index 8cb00aa..7c019f8 100644
--- a/nss/grp-lookup.c
+++ b/nss/grp-lookup.c
@@ -16,7 +16,13 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#include <config.h>
+
 #define DATABASE_NAME group
-#define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
 
 #include "XXX-lookup.c"
diff --git a/nss/nsswitch.c b/nss/nsswitch.c
index 8f31658..f6365a9 100644
--- a/nss/nsswitch.c
+++ b/nss/nsswitch.c
@@ -40,6 +40,13 @@
 #include "nsswitch.h"
 #include "../nscd/nscd_proto.h"
 #include <sysdep.h>
+#include <config.h>
+
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
 
 /* Prototypes for the local functions.  */
 static name_database *nss_parse_file (const char *fname) internal_function;
@@ -848,8 +855,8 @@ __nss_disable_nscd (void (*cb) (size_t, struct traced_file *))
   is_nscd = true;
 
   /* Find all the relevant modules so that the init functions are called.  */
-  nss_load_all_libraries ("passwd", "compat [NOTFOUND=return] files");
-  nss_load_all_libraries ("group", "compat [NOTFOUND=return] files");
+  nss_load_all_libraries ("passwd", DEFAULT_CONFIG);
+  nss_load_all_libraries ("group", DEFAULT_CONFIG);
   nss_load_all_libraries ("hosts", "dns [!UNAVAIL=return] files");
   nss_load_all_libraries ("services", NULL);
 
diff --git a/nss/pwd-lookup.c b/nss/pwd-lookup.c
index 00040d4..180ac7c 100644
--- a/nss/pwd-lookup.c
+++ b/nss/pwd-lookup.c
@@ -16,7 +16,13 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#include <config.h>
+
 #define DATABASE_NAME passwd
-#define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
 
 #include "XXX-lookup.c"
diff --git a/nss/spwd-lookup.c b/nss/spwd-lookup.c
index 319a7bb..f12156d 100644
--- a/nss/spwd-lookup.c
+++ b/nss/spwd-lookup.c
@@ -16,8 +16,14 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#include <config.h>
+
 #define DATABASE_NAME shadow
 #define ALTERNATE_NAME passwd
-#define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
 
 #include "XXX-lookup.c"

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[DJ Delorie]

I find the slight inconsistency in the use of [NOTFOUND=return] in
initgroups.c disconcerting, but IMHO that's irrelevent for this
particular change.

LGTM.

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[Steve Ellcey]

On Tue, 2017-08-22 at 17:07 -0400, DJ Delorie wrote:
> I find the slight inconsistency in the use of [NOTFOUND=return] in
> initgroups.c disconcerting, but IMHO that's irrelevent for this
> particular change.
> 
> LGTM.

Joseph, do you have any issues with the latest version of this patch?
I don't want to go ahead unless you think this has addressed the issues
you had with the initial patch.

Steve Ellcey
sellcey@cavium.com

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[Joseph Myers]

On Mon, 28 Aug 2017, Steve Ellcey wrote:

> On Tue, 2017-08-22 at 17:07 -0400, DJ Delorie wrote:
> > I find the slight inconsistency in the use of [NOTFOUND=return] in
> > initgroups.c disconcerting, but IMHO that's irrelevent for this
> > particular change.
> > 
> > LGTM.
> 
> Joseph, do you have any issues with the latest version of this patch?
> I don't want to go ahead unless you think this has addressed the issues
> you had with the initial patch.

I think the documentation in the manual needs to be updated.  
Specifically, nss.texi documents the defaults for various NSS databases, 
and it needs to be made to reflect how the defaults depend on how glibc is 
configured.

Does the "nis [NOTFOUND=return] files" default in 
nsswitch.c:__nss_database_lookup need to change as well?

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[Steve Ellcey]

On Mon, 2017-08-28 at 20:47 +0000, Joseph Myers wrote:
> On Mon, 28 Aug 2017, Steve Ellcey wrote:
> 
> I think the documentation in the manual needs to be updated.  
> Specifically, nss.texi documents the defaults for various NSS databases, 
> and it needs to be made to reflect how the defaults depend on how glibc is 
> configured.

OK, I can update the documentation.

> Does the "nis [NOTFOUND=return] files" default in 
> nsswitch.c:__nss_database_lookup need to change as well?

I think this default is still OK.  The other changes I made were to
remove 'compat' entries from the list of databases to be searched
if the compat library is not built.  I am not sure why we would want to
change this one.  Am I missing something?

Steve Ellcey
sellcey@cavium.com

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[Joseph Myers]

[-- Attachment #1: Type: text/plain, Size: 663 bytes --]

On Mon, 28 Aug 2017, Steve Ellcey wrote:

> > Does the "nis [NOTFOUND=return] files" default in 
> > nsswitch.c:__nss_database_lookup need to change as well?
> 
> I think this default is still OK.  The other changes I made were to
> remove 'compat' entries from the list of databases to be searched
> if the compat library is not built.  I am not sure why we would want to
> change this one.  Am I missing something?

Well, libnss_nis isn't built by default either.  Is there some reason for 
searching nis by default without the relevant library, but not doing so 
for compat without the relevant library?

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[Steve Ellcey]

[-- Attachment #1: Type: text/plain, Size: 2162 bytes --]

On Mon, 2017-08-28 at 22:57 +0000, Joseph Myers wrote:
> On Mon, 28 Aug 2017, Steve Ellcey wrote:
> 
> > 
> > > 
> > > Does the "nis [NOTFOUND=return] files" default in 
> > > nsswitch.c:__nss_database_lookup need to change as well?
> > I think this default is still OK.  The other changes I made were to
> > remove 'compat' entries from the list of databases to be searched
> > if the compat library is not built.  I am not sure why we would
> > want to
> > change this one.  Am I missing something?
> Well, libnss_nis isn't built by default either.  Is there some reason for 
> searching nis by default without the relevant library, but not doing so 
> for compat without the relevant library?

No reason, I just didn't realise libnss_nis wasn't built by default.

Here is an updated patch for setting the default lookup methods in nss.
I updated nss/nsswitch.c to not use nis lookups when RPC is not
configured and I updated DEFAULT_CONFIG in grp/initgroups.c to be
"compat [NOTFOUND=return] files" when using NSL instead of just "compat
files".  DJ noticed that and I think the missing NOTFOUND is just an
oversight and it conflicted with the documentation in manual/nss.texi.
Fixing the code seemed more reasonable than documenting the existing
setting.  I did update the documentation to reflect the behavour of
this patch.

I tested once with "--enable-obsolete-nsl --enable-obsolete-rpc" and
once without and had no regressions from the unmodified glibc on
aarch64.

OK to checkin?

Steve Ellcey
sellcey@cavium.com


2017-08-29  Steve Ellcey  <sellcey@cavium.com>

	* grp/initgroups.c: Include config.h.
	(DEFAULT_CONFIG): New macro.
	(internal_getgrouplist): Use DEFAULT_CONFIG.
	* nscd/initgrcache.c (addinitgroupsX): Likewise.
	* nss/nsswitch.c (__nss_disable_nscd): Likewise.
	(DEFAULT_DEFCONFIG): New macro.
	(__nss_database_lookup): Use DEFAULT_DEFCONFIG.
	* nss/grp-lookup.c: Include config.h
	(DEFAULT_CONFIG): Set definition based on LINK_OBSOLETE_NSL.
	* nss/pwd-lookup.c (DEFAULT_CONFIG): Likewise.
	* nss/spwd-lookup.c (DEFAULT_CONFIG): Likewise.
	* manual/nss.texi: Update default values section.



[-- Attachment #2: nss.compat.patch --]
[-- Type: text/x-patch, Size: 6567 bytes --]

diff --git a/grp/initgroups.c b/grp/initgroups.c
index 3e4a3de..45dd391 100644
--- a/grp/initgroups.c
+++ b/grp/initgroups.c
@@ -26,10 +26,16 @@
 #include <sys/types.h>
 #include <nsswitch.h>
 #include <scratch_buffer.h>
+#include <config.h>
 
 #include "../nscd/nscd-client.h"
 #include "../nscd/nscd_proto.h"
 
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
 
 /* Type of the lookup function.  */
 typedef enum nss_status (*initgroups_dyn_function) (const char *, gid_t,
@@ -77,7 +83,7 @@ internal_getgrouplist (const char *user, gid_t group, long int *size,
 				 &__nss_initgroups_database) < 0)
 	{
 	  if (__nss_group_database == NULL)
-	    no_more = __nss_database_lookup ("group", NULL, "compat files",
+	    no_more = __nss_database_lookup ("group", NULL, DEFAULT_CONFIG,
 					     &__nss_group_database);
 
 	  __nss_initgroups_database = __nss_group_database;
diff --git a/manual/nss.texi b/manual/nss.texi
index ee70ad3..d534c26 100644
--- a/manual/nss.texi
+++ b/manual/nss.texi
@@ -318,13 +318,17 @@ The @code{passwd}, @code{group}, and @code{shadow} databases are
 traditionally handled in a special way.  The appropriate files in the
 @file{/etc} directory are read but if an entry with a name starting
 with a @code{+} character is found NIS is used.  This kind of lookup
-remains possible by using the special lookup service @code{compat}
-and the default value for the three databases above is
-@code{compat [NOTFOUND=return] files}.
-
-For all other databases the default value is
-@code{nis [NOTFOUND=return] files}.  This solution gives the best
-chance to be correct since NIS and file based lookups are used.
+remains possible if @theglibc{} was configured with the
+@code{--enable-obsolete-nsl} option and the special lookup service
+@code{compat} is used.  If @theglibc{} was configured with the
+@code{--enable-obsolete-nsl} option the default value for the three
+databases above is @code{compat [NOTFOUND=return] files}.  If the
+@code{--enable-obsolete-nsl} option was not used the default value
+for the services is @code{files}.
+
+For all other databases the default value is @code{files} unless
+@theglibc{} was configured with @code{--enable-obsolete-rpc} option, in
+which case it the default value is @code{nis [NOTFOUND=return] files}.
 
 @cindex optimizing NSS
 A second point is that the user should try to optimize the lookup
diff --git a/nscd/initgrcache.c b/nscd/initgrcache.c
index 4deb483..4114fbe 100644
--- a/nscd/initgrcache.c
+++ b/nscd/initgrcache.c
@@ -25,6 +25,7 @@
 #include <unistd.h>
 #include <sys/mman.h>
 #include <scratch_buffer.h>
+#include <config.h>
 
 #include "dbg_log.h"
 #include "nscd.h"
@@ -34,6 +35,11 @@
 
 #include "../nss/nsswitch.h"
 
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
 
 /* Type of the lookup function.  */
 typedef enum nss_status (*initgroups_dyn_function) (const char *, gid_t,
@@ -85,8 +91,7 @@ addinitgroupsX (struct database_dyn *db, int fd, request_header *req,
   int no_more;
 
   if (group_database == NULL)
-    no_more = __nss_database_lookup ("group", NULL,
-				     "compat [NOTFOUND=return] files",
+    no_more = __nss_database_lookup ("group", NULL, DEFAULT_CONFIG,
 				     &group_database);
   else
     no_more = 0;
diff --git a/nss/grp-lookup.c b/nss/grp-lookup.c
index 8cb00aa..7c019f8 100644
--- a/nss/grp-lookup.c
+++ b/nss/grp-lookup.c
@@ -16,7 +16,13 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#include <config.h>
+
 #define DATABASE_NAME group
-#define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
 
 #include "XXX-lookup.c"
diff --git a/nss/nsswitch.c b/nss/nsswitch.c
index 834bef6..3de085e 100644
--- a/nss/nsswitch.c
+++ b/nss/nsswitch.c
@@ -40,6 +40,19 @@
 #include "nsswitch.h"
 #include "../nscd/nscd_proto.h"
 #include <sysdep.h>
+#include <config.h>
+
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
+
+#ifdef LINK_OBSOLETE_RPC
+# define DEFAULT_DEFCONFIG "nis [NOTFOUND=return] files"
+#else
+# define DEFAULT_DEFCONFIG "files"
+#endif
 
 /* Prototypes for the local functions.  */
 static name_database *nss_parse_file (const char *fname) internal_function;
@@ -153,8 +166,7 @@ __nss_database_lookup (const char *database, const char *alternate_name,
      or null to use the most common default.  */
   if (*ni == NULL)
     {
-      *ni = nss_parse_service_list (defconfig
-				    ?: "nis [NOTFOUND=return] files");
+      *ni = nss_parse_service_list (defconfig ?: DEFAULT_DEFCONFIG);
       if (*ni != NULL)
 	{
 	  /* Record the memory we've just allocated in defconfig_entries list,
@@ -852,8 +864,8 @@ __nss_disable_nscd (void (*cb) (size_t, struct traced_file *))
   is_nscd = true;
 
   /* Find all the relevant modules so that the init functions are called.  */
-  nss_load_all_libraries ("passwd", "compat [NOTFOUND=return] files");
-  nss_load_all_libraries ("group", "compat [NOTFOUND=return] files");
+  nss_load_all_libraries ("passwd", DEFAULT_CONFIG);
+  nss_load_all_libraries ("group", DEFAULT_CONFIG);
   nss_load_all_libraries ("hosts", "dns [!UNAVAIL=return] files");
   nss_load_all_libraries ("services", NULL);
 
diff --git a/nss/pwd-lookup.c b/nss/pwd-lookup.c
index 00040d4..180ac7c 100644
--- a/nss/pwd-lookup.c
+++ b/nss/pwd-lookup.c
@@ -16,7 +16,13 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#include <config.h>
+
 #define DATABASE_NAME passwd
-#define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
 
 #include "XXX-lookup.c"
diff --git a/nss/spwd-lookup.c b/nss/spwd-lookup.c
index 319a7bb..f12156d 100644
--- a/nss/spwd-lookup.c
+++ b/nss/spwd-lookup.c
@@ -16,8 +16,14 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#include <config.h>
+
 #define DATABASE_NAME shadow
 #define ALTERNATE_NAME passwd
-#define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
 
 #include "XXX-lookup.c"

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[Joseph Myers]

[-- Attachment #1: Type: text/plain, Size: 962 bytes --]

On Tue, 29 Aug 2017, Steve Ellcey wrote:

> No reason, I just didn't realise libnss_nis wasn't built by default.
> 
> Here is an updated patch for setting the default lookup methods in nss.
> I updated nss/nsswitch.c to not use nis lookups when RPC is not
> configured and I updated DEFAULT_CONFIG in grp/initgroups.c to be
> "compat [NOTFOUND=return] files" when using NSL instead of just "compat
> files".  DJ noticed that and I think the missing NOTFOUND is just an
> oversight and it conflicted with the documentation in manual/nss.texi.
> Fixing the code seemed more reasonable than documenting the existing
> setting.  I did update the documentation to reflect the behavour of
> this patch.
> 
> I tested once with "--enable-obsolete-nsl --enable-obsolete-rpc" and
> once without and had no regressions from the unmodified glibc on
> aarch64.
> 
> OK to checkin?

I have no further comments on this patch.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[DJ Delorie]

> OK to checkin?

LGTM

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[Andreas Schwab]

On Aug 29 2017, Steve Ellcey <sellcey@cavium.com> wrote:

> diff --git a/nss/nsswitch.c b/nss/nsswitch.c
> index 834bef6..3de085e 100644
> --- a/nss/nsswitch.c
> +++ b/nss/nsswitch.c
> @@ -40,6 +40,19 @@
>  #include "nsswitch.h"
>  #include "../nscd/nscd_proto.h"
>  #include <sysdep.h>
> +#include <config.h>
> +
> +#ifdef LINK_OBSOLETE_NSL
> +# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
> +#else
> +# define DEFAULT_CONFIG "files"
> +#endif
> +
> +#ifdef LINK_OBSOLETE_RPC
> +# define DEFAULT_DEFCONFIG "nis [NOTFOUND=return] files"
> +#else
> +# define DEFAULT_DEFCONFIG "files"
> +#endif

nss_nis is part of obsolete-nsl, not obsolete-rpc.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[Steve Ellcey]

[-- Attachment #1: Type: text/plain, Size: 984 bytes --]

On Thu, 2017-08-31 at 09:20 +0200, Andreas Schwab wrote:
> 
> > +#ifdef LINK_OBSOLETE_RPC
> > +# define DEFAULT_DEFCONFIG "nis [NOTFOUND=return] files"
> > +#else
> > +# define DEFAULT_DEFCONFIG "files"
> > +#endif
> nss_nis is part of obsolete-nsl, not obsolete-rpc.
> 
> Andreas.

Yes it is.  Here is an updated patch with that change.

Steve Ellcey
sellcey@cavium.com

2017-08-31  Steve Ellcey  <sellcey@cavium.com>

	* grp/initgroups.c: Include config.h.
	(DEFAULT_CONFIG): New macro.
	(internal_getgrouplist): Use DEFAULT_CONFIG.
	* nscd/initgrcache.c (addinitgroupsX): Likewise.
	* nss/nsswitch.c (__nss_disable_nscd): Likewise.
	(DEFAULT_DEFCONFIG): New macro.
	(__nss_database_lookup): Use DEFAULT_DEFCONFIG.
	* nss/grp-lookup.c: Include config.h
	(DEFAULT_CONFIG): Set definition based on LINK_OBSOLETE_NSL.
	* nss/pwd-lookup.c (DEFAULT_CONFIG): Likewise.
	* nss/spwd-lookup.c (DEFAULT_CONFIG): Likewise.
	* manual/nss.texi: Update default values section.


[-- Attachment #2: nss.compat.patch --]
[-- Type: text/x-patch, Size: 6530 bytes --]

diff --git a/grp/initgroups.c b/grp/initgroups.c
index 3e4a3de..45dd391 100644
--- a/grp/initgroups.c
+++ b/grp/initgroups.c
@@ -26,10 +26,16 @@
 #include <sys/types.h>
 #include <nsswitch.h>
 #include <scratch_buffer.h>
+#include <config.h>
 
 #include "../nscd/nscd-client.h"
 #include "../nscd/nscd_proto.h"
 
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
 
 /* Type of the lookup function.  */
 typedef enum nss_status (*initgroups_dyn_function) (const char *, gid_t,
@@ -77,7 +83,7 @@ internal_getgrouplist (const char *user, gid_t group, long int *size,
 				 &__nss_initgroups_database) < 0)
 	{
 	  if (__nss_group_database == NULL)
-	    no_more = __nss_database_lookup ("group", NULL, "compat files",
+	    no_more = __nss_database_lookup ("group", NULL, DEFAULT_CONFIG,
 					     &__nss_group_database);
 
 	  __nss_initgroups_database = __nss_group_database;
diff --git a/manual/nss.texi b/manual/nss.texi
index ee70ad3..d534c26 100644
--- a/manual/nss.texi
+++ b/manual/nss.texi
@@ -318,13 +318,17 @@ The @code{passwd}, @code{group}, and @code{shadow} databases are
 traditionally handled in a special way.  The appropriate files in the
 @file{/etc} directory are read but if an entry with a name starting
 with a @code{+} character is found NIS is used.  This kind of lookup
-remains possible by using the special lookup service @code{compat}
-and the default value for the three databases above is
-@code{compat [NOTFOUND=return] files}.
-
-For all other databases the default value is
-@code{nis [NOTFOUND=return] files}.  This solution gives the best
-chance to be correct since NIS and file based lookups are used.
+remains possible if @theglibc{} was configured with the
+@code{--enable-obsolete-nsl} option and the special lookup service
+@code{compat} is used.  If @theglibc{} was configured with the
+@code{--enable-obsolete-nsl} option the default value for the three
+databases above is @code{compat [NOTFOUND=return] files}.  If the
+@code{--enable-obsolete-nsl} option was not used the default value
+for the services is @code{files}.
+
+For all other databases the default value is @code{files} unless
+@theglibc{} was configured with @code{--enable-obsolete-rpc} option, in
+which case it the default value is @code{nis [NOTFOUND=return] files}.
 
 @cindex optimizing NSS
 A second point is that the user should try to optimize the lookup
diff --git a/nscd/initgrcache.c b/nscd/initgrcache.c
index 4deb483..4114fbe 100644
--- a/nscd/initgrcache.c
+++ b/nscd/initgrcache.c
@@ -25,6 +25,7 @@
 #include <unistd.h>
 #include <sys/mman.h>
 #include <scratch_buffer.h>
+#include <config.h>
 
 #include "dbg_log.h"
 #include "nscd.h"
@@ -34,6 +35,11 @@
 
 #include "../nss/nsswitch.h"
 
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
 
 /* Type of the lookup function.  */
 typedef enum nss_status (*initgroups_dyn_function) (const char *, gid_t,
@@ -85,8 +91,7 @@ addinitgroupsX (struct database_dyn *db, int fd, request_header *req,
   int no_more;
 
   if (group_database == NULL)
-    no_more = __nss_database_lookup ("group", NULL,
-				     "compat [NOTFOUND=return] files",
+    no_more = __nss_database_lookup ("group", NULL, DEFAULT_CONFIG,
 				     &group_database);
   else
     no_more = 0;
diff --git a/nss/grp-lookup.c b/nss/grp-lookup.c
index 8cb00aa..7c019f8 100644
--- a/nss/grp-lookup.c
+++ b/nss/grp-lookup.c
@@ -16,7 +16,13 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#include <config.h>
+
 #define DATABASE_NAME group
-#define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
 
 #include "XXX-lookup.c"
diff --git a/nss/nsswitch.c b/nss/nsswitch.c
index 834bef6..78f6687 100644
--- a/nss/nsswitch.c
+++ b/nss/nsswitch.c
@@ -40,6 +40,15 @@
 #include "nsswitch.h"
 #include "../nscd/nscd_proto.h"
 #include <sysdep.h>
+#include <config.h>
+
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG    "compat [NOTFOUND=return] files"
+# define DEFAULT_DEFCONFIG "nis [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG    "files"
+# define DEFAULT_DEFCONFIG "files"
+#endif
 
 /* Prototypes for the local functions.  */
 static name_database *nss_parse_file (const char *fname) internal_function;
@@ -153,8 +162,7 @@ __nss_database_lookup (const char *database, const char *alternate_name,
      or null to use the most common default.  */
   if (*ni == NULL)
     {
-      *ni = nss_parse_service_list (defconfig
-				    ?: "nis [NOTFOUND=return] files");
+      *ni = nss_parse_service_list (defconfig ?: DEFAULT_DEFCONFIG);
       if (*ni != NULL)
 	{
 	  /* Record the memory we've just allocated in defconfig_entries list,
@@ -852,8 +860,8 @@ __nss_disable_nscd (void (*cb) (size_t, struct traced_file *))
   is_nscd = true;
 
   /* Find all the relevant modules so that the init functions are called.  */
-  nss_load_all_libraries ("passwd", "compat [NOTFOUND=return] files");
-  nss_load_all_libraries ("group", "compat [NOTFOUND=return] files");
+  nss_load_all_libraries ("passwd", DEFAULT_CONFIG);
+  nss_load_all_libraries ("group", DEFAULT_CONFIG);
   nss_load_all_libraries ("hosts", "dns [!UNAVAIL=return] files");
   nss_load_all_libraries ("services", NULL);
 
diff --git a/nss/pwd-lookup.c b/nss/pwd-lookup.c
index 00040d4..180ac7c 100644
--- a/nss/pwd-lookup.c
+++ b/nss/pwd-lookup.c
@@ -16,7 +16,13 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#include <config.h>
+
 #define DATABASE_NAME passwd
-#define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
 
 #include "XXX-lookup.c"
diff --git a/nss/spwd-lookup.c b/nss/spwd-lookup.c
index 319a7bb..f12156d 100644
--- a/nss/spwd-lookup.c
+++ b/nss/spwd-lookup.c
@@ -16,8 +16,14 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#include <config.h>
+
 #define DATABASE_NAME shadow
 #define ALTERNATE_NAME passwd
-#define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#ifdef LINK_OBSOLETE_NSL
+# define DEFAULT_CONFIG "compat [NOTFOUND=return] files"
+#else
+# define DEFAULT_CONFIG "files"
+#endif
 
 #include "XXX-lookup.c"

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[Steve Ellcey]

Ping.  Any other comments on the last version of this patch?
Does it look OK to check in?

Steve Ellcey
sellcey@cavium.com

On Thu, 2017-08-31 at 08:24 -0700, Steve Ellcey wrote:
> On Thu, 2017-08-31 at 09:20 +0200, Andreas Schwab wrote:
> > 
> >  
> > > 
> > > +#ifdef LINK_OBSOLETE_RPC
> > > +# define DEFAULT_DEFCONFIG "nis [NOTFOUND=return] files"
> > > +#else
> > > +# define DEFAULT_DEFCONFIG "files"
> > > +#endif
> > nss_nis is part of obsolete-nsl, not obsolete-rpc.
> > 
> > Andreas.
> Yes it is.  Here is an updated patch with that change.
> 
> Steve Ellcey
> sellcey@cavium.com
> 
> 2017-08-31  Steve Ellcey  <sellcey@cavium.com>
> 
> 	* grp/initgroups.c: Include config.h.
> 	(DEFAULT_CONFIG): New macro.
> 	(internal_getgrouplist): Use DEFAULT_CONFIG.
> 	* nscd/initgrcache.c (addinitgroupsX): Likewise.
> 	* nss/nsswitch.c (__nss_disable_nscd): Likewise.
> 	(DEFAULT_DEFCONFIG): New macro.
> 	(__nss_database_lookup): Use DEFAULT_DEFCONFIG.
> 	* nss/grp-lookup.c: Include config.h
> 	(DEFAULT_CONFIG): Set definition based on LINK_OBSOLETE_NSL.
> 	* nss/pwd-lookup.c (DEFAULT_CONFIG): Likewise.
> 	* nss/spwd-lookup.c (DEFAULT_CONFIG): Likewise.
> 	* manual/nss.texi: Update default values section.
> 

Re: [PATCH] Remove compat from DEFAULT_CONFIG lookup strings

[DJ Delorie]

Steve Ellcey <sellcey@cavium.com> writes:
> Yes it is.  Here is an updated patch with that change.

LGTM