[Patch] Fix nss/nss_test1.c compile with latest GCC

[Patch] Fix nss/nss_test1.c compile with latest GCC

[Steve Ellcey]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1165 bytes --]


While building and testing glibc with the latest (ToT) GCC, I got the
following error message:

nss_test1.c:60:46: error: division ‘sizeof (struct passwd *) / sizeof (struct passwd)’ does not compute the number of array elements [-Werror=sizeof-pointer-div]
 #define default_npwd_data (sizeof (pwd_data) / sizeof (pwd_data[0]))

I think this is due to new error checking added to GCC and in this
case I think that GCC is correct in its error.  We should
be using the default_pwd_data in this expression and not pwd_data.

This patch fixes the proglem, OK for checkin?


2017-07-19  Steve Ellcey  <sellcey@cavium.com>

	* nss/nss_test1.c (default_npwd_data): Fix definition.



diff --git a/nss/nss_test1.c b/nss/nss_test1.c
index b728e41..86bbc2c 100644
--- a/nss/nss_test1.c
+++ b/nss/nss_test1.c
@@ -57,7 +57,8 @@ static struct passwd default_pwd_data[] =
     PWD (60),
     PWD (20000)
   };
-#define default_npwd_data (sizeof (pwd_data) / sizeof (pwd_data[0]))
+#define default_npwd_data \
+  (sizeof (default_pwd_data) / sizeof (default_pwd_data[0]))
 
 static struct passwd *pwd_data = default_pwd_data;
 static int npwd_data = default_npwd_data;

Re: [Patch] Fix nss/nss_test1.c compile with latest GCC

[DJ Delorie]

Steve Ellcey <sellcey@cavium.com> writes:
> I think this is due to new error checking added to GCC and in this
> case I think that GCC is correct in its error.  We should
> be using the default_pwd_data in this expression and not pwd_data.

Correct, although no existing tests actually use that data, so the bug
(and the fix) do not affect test results.

> This patch fixes the proglem, OK for checkin?

I was going to check in the same patch... but my rawhide gcc didn't give
a warning so I haddn't yet confirmed it.

So LGTM :-)

Thanks!

Re: [Patch] Fix nss/nss_test1.c compile with latest GCC

[Steve Ellcey]

On Wed, 2017-07-19 at 19:13 -0400, DJ Delorie wrote:
> Steve Ellcey <sellcey@cavium.com> writes:
> > 
> > I think this is due to new error checking added to GCC and in this
> > case I think that GCC is correct in its error.  We should
> > be using the default_pwd_data in this expression and not pwd_data.
> Correct, although no existing tests actually use that data, so the
> bug
> (and the fix) do not affect test results.
> 
> > 
> > This patch fixes the proglem, OK for checkin?
> I was going to check in the same patch... but my rawhide gcc didn't
> give
> a warning so I haddn't yet confirmed it.
> 
> So LGTM :-)
> 
> Thanks!

Since there haven't seen any objections to this I am going to go ahead
and check it in.

Steve Ellcey

Re: [Patch] Fix nss/nss_test1.c compile with latest GCC

[H.J. Lu]

On Fri, Jul 21, 2017 at 9:54 AM, Steve Ellcey <sellcey@cavium.com> wrote:
> On Wed, 2017-07-19 at 19:13 -0400, DJ Delorie wrote:
>> Steve Ellcey <sellcey@cavium.com> writes:
>> >
>> > I think this is due to new error checking added to GCC and in this
>> > case I think that GCC is correct in its error.  We should
>> > be using the default_pwd_data in this expression and not pwd_data.
>> Correct, although no existing tests actually use that data, so the
>> bug
>> (and the fix) do not affect test results.
>>
>> >
>> > This patch fixes the proglem, OK for checkin?
>> I was going to check in the same patch... but my rawhide gcc didn't
>> give
>> a warning so I haddn't yet confirmed it.
>>
>> So LGTM :-)
>>
>> Thanks!
>
> Since there haven't seen any objections to this I am going to go ahead
> and check it in.
>

On Fedora 26, this caused:

[hjl@gnu-tools-1 build-x86_64-linux]$ nss/tst-nss-test5
[1] passwd entry 30.name30 had uid 300
[1] passwd entry 30.name30 had pw_name "name300" (expected "name30")
    passwd 300.name300 (*) : 100, *, *, *

    passwd 30.name30 (*) : 100, *, *, *

[2] passwd pw_name for 200.name200 was (null)
    passwd 200.(null) (*) : 100, *, *, *

    passwd 200.name200 (*) : 100, *, *, *

[3] passwd pw_name for 60.(null) was name60
    passwd 60.name60 (*) : 100, *, *, *

    passwd 60.(null) (*) : 100, *, *, *

[4] passwd entry 20000.name20000 had gid 200 (expected 100)
    passwd 20000.name20000 (*) : 200, *, *, *

    passwd 20000.name20000 (*) : 100, *, *, *

[5] passwd entry 2.name2 had gid 2 (expected 100)
[5] passwd entry 2.name2 had pw_passwd "x" (expected "*")
[5] passwd entry 2.name2 had pw_gecos "y" (expected "*")
[5] passwd entry 2.name2 had pw_dir "z" (expected "*")
    passwd 2.name2 (x) : 2, y, z, *

    passwd 2.name2 (*) : 100, *, *, *

FAIL: [?] passwd entry 30.name30 unexpected
FAIL: Found 10 errors, expected 9
[hjl@gnu-tools-1 build-x86_64-linux]$



-- 
H.J.

Re: [Patch] Fix nss/nss_test1.c compile with latest GCC

[H.J. Lu]

On Fri, Jul 21, 2017 at 04:02:15PM -0700, H.J. Lu wrote:
> On Fri, Jul 21, 2017 at 9:54 AM, Steve Ellcey <sellcey@cavium.com> wrote:
> > On Wed, 2017-07-19 at 19:13 -0400, DJ Delorie wrote:
> >> Steve Ellcey <sellcey@cavium.com> writes:
> >> >
> >> > I think this is due to new error checking added to GCC and in this
> >> > case I think that GCC is correct in its error.  We should
> >> > be using the default_pwd_data in this expression and not pwd_data.
> >> Correct, although no existing tests actually use that data, so the
> >> bug
> >> (and the fix) do not affect test results.
> >>
> >> >
> >> > This patch fixes the proglem, OK for checkin?
> >> I was going to check in the same patch... but my rawhide gcc didn't
> >> give
> >> a warning so I haddn't yet confirmed it.
> >>
> >> So LGTM :-)
> >>
> >> Thanks!
> >
> > Since there haven't seen any objections to this I am going to go ahead
> > and check it in.
> >
> 
> On Fedora 26, this caused:
> 
> [hjl@gnu-tools-1 build-x86_64-linux]$ nss/tst-nss-test5
> [1] passwd entry 30.name30 had uid 300
> [1] passwd entry 30.name30 had pw_name "name300" (expected "name30")
>     passwd 300.name300 (*) : 100, *, *, *
> 
>     passwd 30.name30 (*) : 100, *, *, *
> 
> [2] passwd pw_name for 200.name200 was (null)
>     passwd 200.(null) (*) : 100, *, *, *
> 
>     passwd 200.name200 (*) : 100, *, *, *
> 
> [3] passwd pw_name for 60.(null) was name60
>     passwd 60.name60 (*) : 100, *, *, *
> 
>     passwd 60.(null) (*) : 100, *, *, *
> 
> [4] passwd entry 20000.name20000 had gid 200 (expected 100)
>     passwd 20000.name20000 (*) : 200, *, *, *
> 
>     passwd 20000.name20000 (*) : 100, *, *, *
> 
> [5] passwd entry 2.name2 had gid 2 (expected 100)
> [5] passwd entry 2.name2 had pw_passwd "x" (expected "*")
> [5] passwd entry 2.name2 had pw_gecos "y" (expected "*")
> [5] passwd entry 2.name2 had pw_dir "z" (expected "*")
>     passwd 2.name2 (x) : 2, y, z, *
> 
>     passwd 2.name2 (*) : 100, *, *, *
> 
> FAIL: [?] passwd entry 30.name30 unexpected
> FAIL: Found 10 errors, expected 9
> [hjl@gnu-tools-1 build-x86_64-linux]$

Remove default_pwd_data fixes the problem for me and all nss tests
passed.  Is it really needed?


H.J.
---
diff --git a/nss/nss_test1.c b/nss/nss_test1.c
index 86bbc2c548..47ac21001e 100644
--- a/nss/nss_test1.c
+++ b/nss/nss_test1.c
@@ -43,25 +43,8 @@
 #include "nss_test.h"
 
 /* -------------------------------------------------- */
-/* Default Data.  */
-
-static struct passwd default_pwd_data[] =
-  {
-#define PWD(u) \
-    { .pw_name = (char *) "name" #u, .pw_passwd = (char *) "*", .pw_uid = u,  \
-      .pw_gid = 100, .pw_gecos = (char *) "*", .pw_dir = (char *) "*",	      \
-      .pw_shell = (char *) "*" }
-    PWD (30),
-    PWD (100),
-    PWD (200),
-    PWD (60),
-    PWD (20000)
-  };
-#define default_npwd_data \
-  (sizeof (default_pwd_data) / sizeof (default_pwd_data[0]))
-
-static struct passwd *pwd_data = default_pwd_data;
-static int npwd_data = default_npwd_data;
+static struct passwd *pwd_data = NULL;
+static int npwd_data = 0;
 
 static struct group *grp_data = NULL;
 static int ngrp_data = 0;

Re: [Patch] Fix nss/nss_test1.c compile with latest GCC

[DJ Delorie]

I think the right solution is to fix test5.  I suspect that leaving in
"bad" default data is a good test for a properly configured test,a and
we might want to add some "bad" default group data as well.

diff --git a/nss/tst-nss-test5.c b/nss/tst-nss-test5.c
index b70f21e..fef41f0 100644
--- a/nss/tst-nss-test5.c
+++ b/nss/tst-nss-test5.c
@@ -65,7 +65,7 @@ do_test (void)
   int i;
   struct passwd *p;
 
-  __nss_configure_lookup ("passwd", "test1 test2");
+  __nss_configure_lookup ("passwd", "test1");
 
   setpwent ();
 

Re: [Patch] Fix nss/nss_test1.c compile with latest GCC

[H.J. Lu]

On Fri, Jul 21, 2017 at 4:23 PM, DJ Delorie <dj@redhat.com> wrote:
>
> I think the right solution is to fix test5.  I suspect that leaving in
> "bad" default data is a good test for a properly configured test,a and
> we might want to add some "bad" default group data as well.
>
> diff --git a/nss/tst-nss-test5.c b/nss/tst-nss-test5.c
> index b70f21e..fef41f0 100644
> --- a/nss/tst-nss-test5.c
> +++ b/nss/tst-nss-test5.c
> @@ -65,7 +65,7 @@ do_test (void)
>    int i;
>    struct passwd *p;
>
> -  __nss_configure_lookup ("passwd", "test1 test2");
> +  __nss_configure_lookup ("passwd", "test1");
>
>    setpwent ();
>

Works for me.  Can you check it in?

Thanks.

-- 
H.J.

Re: [Patch] Fix nss/nss_test1.c compile with latest GCC

[DJ Delorie]

"H.J. Lu" <hjl.tools@gmail.com> writes:
> Works for me.  Can you check it in?

Done.

Re: [Patch] Fix nss/nss_test1.c compile with latest GCC

[Steve Ellcey]

On Fri, 2017-07-21 at 19:51 -0400, DJ Delorie wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
> > 
> > Works for me.  Can you check it in?
> Done.

Thanks DJ, sorry I missed the run time failure when fixing the build
failure.

Steve Ellcey
sellcey@cavium.com