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

Message ID e3bcc857-3af7-829f-d0fb-a5a6e29f6327@gmail.com
State Committed
Headers
Series adjust RPC function declarations to match Sun's (BZ 26686) |

Commit Message

Martin Sebor Oct. 5, 2020, 9:17 p.m. UTC
  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
  

Comments

Florian Weimer Oct. 6, 2020, 11:13 a.m. UTC | #1
* 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
  
Martin Sebor Oct. 6, 2020, 8:21 p.m. UTC | #2
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
  
Florian Weimer Oct. 6, 2020, 8:51 p.m. UTC | #3
* 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
  
Martin Sebor Oct. 8, 2020, 6:57 p.m. UTC | #4
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
>
  

Patch

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);
 
 /*
  *