[PATCH] adjust RPC function declarations to match Sun's (BZ 26686)

[PATCH] adjust RPC function declarations to match Sun's (BZ 26686)

[Martin Sebor]

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

Building Glibc with the latest GCC 11 shows a number of instances
of the new -Warray-parameter.  The warning is designed to encourage
consistency in the forms of array arguments in redeclarations of
the same function (and, ultimately, to enable the detection of out
of bounds accesses via such arguments).

To avoid the subset of these warnings for the RPC APIs, the attached
patch changes the declarations of these functions to match both their
definitions and the Oracle RPC documentation.

Besides avoiding the -Warray-parameter warnings the effect of this
change is for GCC to issue warnings when either the functions are
passed an array with fewer than MAXNETNAMELEN + 1 elements, or when
the functions themselves access elements outside the array bounds.

I tested the patch by building Glibc with GCC and confirming
the warnings are gone, and by running the tests and confirming no
new failures in the test suite.

Martin

[-- Attachment #2: glibc-26686.diff --]
[-- Type: text/x-patch, Size: 1048 bytes --]

diff --git a/sunrpc/rpc/auth.h b/sunrpc/rpc/auth.h
index e01b077214..6302e6e83b 100644
--- a/sunrpc/rpc/auth.h
+++ b/sunrpc/rpc/auth.h
@@ -179,12 +179,15 @@ extern AUTH *authdes_pk_create (const char *, netobj *, u_int,
  *  Netname manipulating functions
  *
  */
-extern int getnetname (char *) __THROW;
-extern int host2netname (char *, const char *, const char *) __THROW;
-extern int user2netname (char *, const uid_t, const char *) __THROW;
-extern int netname2user (const char *, uid_t *, gid_t *, int *, gid_t *)
-     __THROW;
-extern int netname2host (const char *, char *, const int) __THROW;
+extern int getnetname (char [MAXNETNAMELEN + 1]) __THROW;
+extern int host2netname (char [MAXNETNAMELEN + 1], const char *,
+			 const char *) __THROW;
+extern int user2netname (char [MAXNETNAMELEN + 1], const uid_t,
+			 const char *) __THROW;
+extern int netname2user (const char [MAXNETNAMELEN + 1], uid_t *, gid_t *,
+			 int *, gid_t *) __THROW;
+extern int netname2host (const char[MAXNETNAMELEN + 1], char *,
+			 const int);
 
 /*
  *

Re: [PATCH] adjust RPC function declarations to match Sun's (BZ 26686)

[Florian Weimer]

* Martin Sebor via Libc-alpha:

> diff --git a/sunrpc/rpc/auth.h b/sunrpc/rpc/auth.h
> index e01b077214..6302e6e83b 100644
> --- a/sunrpc/rpc/auth.h
> +++ b/sunrpc/rpc/auth.h
> @@ -179,12 +179,15 @@ extern AUTH *authdes_pk_create (const char *, netobj *, u_int,
>   *  Netname manipulating functions
>   *
>   */
> -extern int getnetname (char *) __THROW;
> -extern int host2netname (char *, const char *, const char *) __THROW;
> -extern int user2netname (char *, const uid_t, const char *) __THROW;
> -extern int netname2user (const char *, uid_t *, gid_t *, int *, gid_t *)
> -     __THROW;
> -extern int netname2host (const char *, char *, const int) __THROW;
> +extern int getnetname (char [MAXNETNAMELEN + 1]) __THROW;
> +extern int host2netname (char [MAXNETNAMELEN + 1], const char *,
> +			 const char *) __THROW;
> +extern int user2netname (char [MAXNETNAMELEN + 1], const uid_t,
> +			 const char *) __THROW;
> +extern int netname2user (const char [MAXNETNAMELEN + 1], uid_t *, gid_t *,
> +			 int *, gid_t *) __THROW;
> +extern int netname2host (const char[MAXNETNAMELEN + 1], char *,
> +			 const int);

I think for the read-only strings (the const arguments), you need to fix
the implementation, not the public header.  It's perfectly fine to use
these functions with shorter strings, I believe.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH] adjust RPC function declarations to match Sun's (BZ 26686)

[Martin Sebor]

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

On 10/6/20 5:13 AM, Florian Weimer wrote:
> * Martin Sebor via Libc-alpha:
> 
>> diff --git a/sunrpc/rpc/auth.h b/sunrpc/rpc/auth.h
>> index e01b077214..6302e6e83b 100644
>> --- a/sunrpc/rpc/auth.h
>> +++ b/sunrpc/rpc/auth.h
>> @@ -179,12 +179,15 @@ extern AUTH *authdes_pk_create (const char *, netobj *, u_int,
>>    *  Netname manipulating functions
>>    *
>>    */
>> -extern int getnetname (char *) __THROW;
>> -extern int host2netname (char *, const char *, const char *) __THROW;
>> -extern int user2netname (char *, const uid_t, const char *) __THROW;
>> -extern int netname2user (const char *, uid_t *, gid_t *, int *, gid_t *)
>> -     __THROW;
>> -extern int netname2host (const char *, char *, const int) __THROW;
>> +extern int getnetname (char [MAXNETNAMELEN + 1]) __THROW;
>> +extern int host2netname (char [MAXNETNAMELEN + 1], const char *,
>> +			 const char *) __THROW;
>> +extern int user2netname (char [MAXNETNAMELEN + 1], const uid_t,
>> +			 const char *) __THROW;
>> +extern int netname2user (const char [MAXNETNAMELEN + 1], uid_t *, gid_t *,
>> +			 int *, gid_t *) __THROW;
>> +extern int netname2host (const char[MAXNETNAMELEN + 1], char *,
>> +			 const int);
> 
> I think for the read-only strings (the const arguments), you need to fix
> the implementation, not the public header.  It's perfectly fine to use
> these functions with shorter strings, I believe.

You're right.  I've changed the signatures in the definitions of these
last two functions instead.

Martin

[-- Attachment #2: glibc-26686.diff --]
[-- Type: text/x-patch, Size: 1628 bytes --]

diff --git a/sunrpc/netname.c b/sunrpc/netname.c
index 61d82ca31a..24ee519e42 100644
--- a/sunrpc/netname.c
+++ b/sunrpc/netname.c
@@ -142,7 +142,7 @@ typedef int (*netname2user_function) (const char netname[MAXNETNAMELEN + 1],
 				      uid_t *, gid_t *, int *, gid_t *);
 
 int
-netname2user (const char netname[MAXNETNAMELEN + 1], uid_t * uidp, gid_t * gidp,
+netname2user (const char *netname, uid_t * uidp, gid_t * gidp,
 	      int *gidlenp, gid_t * gidlist)
 {
   static service_user *startp;
@@ -189,8 +189,7 @@ libc_hidden_nolink_sunrpc (netname2user, GLIBC_2_1)
 #endif
 
 int
-netname2host (const char netname[MAXNETNAMELEN + 1], char *hostname,
-	      const int hostlen)
+netname2host (const char *netname, char *hostname, const int hostlen)
 {
   char *p1, *p2;
 
diff --git a/sunrpc/rpc/auth.h b/sunrpc/rpc/auth.h
index e01b077214..0b46408890 100644
--- a/sunrpc/rpc/auth.h
+++ b/sunrpc/rpc/auth.h
@@ -179,9 +179,11 @@ extern AUTH *authdes_pk_create (const char *, netobj *, u_int,
  *  Netname manipulating functions
  *
  */
-extern int getnetname (char *) __THROW;
-extern int host2netname (char *, const char *, const char *) __THROW;
-extern int user2netname (char *, const uid_t, const char *) __THROW;
+extern int getnetname (char [MAXNETNAMELEN + 1]) __THROW;
+extern int host2netname (char [MAXNETNAMELEN + 1], const char *,
+			 const char *) __THROW;
+extern int user2netname (char [MAXNETNAMELEN + 1], const uid_t,
+			 const char *) __THROW;
 extern int netname2user (const char *, uid_t *, gid_t *, int *, gid_t *)
      __THROW;
 extern int netname2host (const char *, char *, const int) __THROW;

Re: [PATCH] adjust RPC function declarations to match Sun's (BZ 26686)

[Florian Weimer]

* Martin Sebor:

>> I think for the read-only strings (the const arguments), you need to
>> fix
>> the implementation, not the public header.  It's perfectly fine to use
>> these functions with shorter strings, I believe.
>
> You're right.  I've changed the signatures in the definitions of these
> last two functions instead.

Looks good now, thanks.  Is the commit subject still correct?  Can you
change it to (addung “sunrpc”):

sunrpc: Adjust RPC function declarations to match Sun's (bug 26686)

I'm not sure if the tooling now recognizes “(bug 26686)”, but it's the
syntax that we used before.  [BZ #26686] would work as well.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH] adjust RPC function declarations to match Sun's (BZ 26686)

[Martin Sebor]

On 10/6/20 2:51 PM, Florian Weimer wrote:
> * Martin Sebor:
> 
>>> I think for the read-only strings (the const arguments), you need to
>>> fix
>>> the implementation, not the public header.  It's perfectly fine to use
>>> these functions with shorter strings, I believe.
>>
>> You're right.  I've changed the signatures in the definitions of these
>> last two functions instead.
> 
> Looks good now, thanks.  Is the commit subject still correct?  Can you
> change it to (addung “sunrpc”):
> 
> sunrpc: Adjust RPC function declarations to match Sun's (bug 26686)

Sure.  I've just pushed the patch with this subject.

Thanks
Martin

> 
> I'm not sure if the tooling now recognizes “(bug 26686)”, but it's the
> syntax that we used before.  [BZ #26686] would work as well.
> 
> Thanks,
> Florian
>