diff mbox series

[5/6] sunrpc: Suppress GCC -Os warning on user2netname

Message ID 20220921135108.3324737-6-adhemerval.zanella@linaro.org
State Committed
Commit 6128e82ebe973163d2dd614d31753c88c0c4d645
Headers
Series Fix -Os build |

Commit Message

Adhemerval Zanella Sept. 21, 2022, 1:51 p.m. UTC 
  GCC with -Os warns that sprint might overflow:

  netname.c: In function ‘user2netname’:
  netname.c:51:28: error: ‘%s’ directive writing up to 255 bytes into a
  region of size between 239 and 249 [-Werror=format-overflow=]
     51 |   sprintf (netname, "%s.%d@%s", OPSYS, uid, dfltdom);
        |                            ^~               ~~~~~~~
  netname.c:51:3: note: ‘sprintf’ output between 8 and 273 bytes into a
  destination of size 256
     51 |   sprintf (netname, "%s.%d@%s", OPSYS, uid, dfltdom);
        |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  cc1: all warnings being treated as errors

However the code does test prior the sprintf call that dfltdom plus
the required extra space for OPSYS, uid, and extra character will not
overflow and return 0 instead.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 sunrpc/netname.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Carlos O'Donell Oct. 5, 2022, 1:52 p.m. UTC | #1 
On Wed, Sep 21, 2022 at 10:51:07AM -0300, Adhemerval Zanella via Libc-alpha wrote:
> GCC with -Os warns that sprint might overflow:
> 
>   netname.c: In function ‘user2netname’:
>   netname.c:51:28: error: ‘%s’ directive writing up to 255 bytes into a
>   region of size between 239 and 249 [-Werror=format-overflow=]
>      51 |   sprintf (netname, "%s.%d@%s", OPSYS, uid, dfltdom);
>         |                            ^~               ~~~~~~~
>   netname.c:51:3: note: ‘sprintf’ output between 8 and 273 bytes into a
>   destination of size 256
>      51 |   sprintf (netname, "%s.%d@%s", OPSYS, uid, dfltdom);
>         |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   cc1: all warnings being treated as errors
> 
> However the code does test prior the sprintf call that dfltdom plus
> the required extra space for OPSYS, uid, and extra character will not
> overflow and return 0 instead.

LGTM.

No regressions on x86_64.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
Tested-by: Carlos O'Donell <carlos@redhat.com>

> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  sunrpc/netname.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/sunrpc/netname.c b/sunrpc/netname.c
> index bf7f0b81c4..c1d1c43e50 100644
> --- a/sunrpc/netname.c
> +++ b/sunrpc/netname.c
> @@ -20,6 +20,7 @@
>  #include <string.h>
>  #include <rpc/rpc.h>
>  #include <shlib-compat.h>
> +#include <libc-diag.h>

OK.

>  
>  #include "nsswitch.h"
>  
> @@ -48,7 +49,12 @@ user2netname (char netname[MAXNETNAMELEN + 1], const uid_t uid,
>    if ((strlen (dfltdom) + OPSYS_LEN + 3 + MAXIPRINT) > (size_t) MAXNETNAMELEN)
>      return 0;
>  
> +  /* GCC with -Os warns that sprint might overflow while handling dfltdom,
> +     however the above test does check if an overflow would happen.  */
> +  DIAG_PUSH_NEEDS_COMMENT;
> +  DIAG_IGNORE_Os_NEEDS_COMMENT (8, "-Wformat-overflow");
>    sprintf (netname, "%s.%d@%s", OPSYS, uid, dfltdom);
> +  DIAG_POP_NEEDS_COMMENT;

OK. Agreed. The code above does that check.

>    i = strlen (netname);
>    if (netname[i - 1] == '.')
>      netname[i - 1] = '\0';
> -- 
> 2.34.1
>
diff mbox series

Patch

diff --git a/sunrpc/netname.c b/sunrpc/netname.c
index bf7f0b81c4..c1d1c43e50 100644
--- a/sunrpc/netname.c
+++ b/sunrpc/netname.c
@@ -20,6 +20,7 @@ 
 #include <string.h>
 #include <rpc/rpc.h>
 #include <shlib-compat.h>
+#include <libc-diag.h>
 
 #include "nsswitch.h"
 
@@ -48,7 +49,12 @@  user2netname (char netname[MAXNETNAMELEN + 1], const uid_t uid,
   if ((strlen (dfltdom) + OPSYS_LEN + 3 + MAXIPRINT) > (size_t) MAXNETNAMELEN)
     return 0;
 
+  /* GCC with -Os warns that sprint might overflow while handling dfltdom,
+     however the above test does check if an overflow would happen.  */
+  DIAG_PUSH_NEEDS_COMMENT;
+  DIAG_IGNORE_Os_NEEDS_COMMENT (8, "-Wformat-overflow");
   sprintf (netname, "%s.%d@%s", OPSYS, uid, dfltdom);
+  DIAG_POP_NEEDS_COMMENT;
   i = strlen (netname);
   if (netname[i - 1] == '.')
     netname[i - 1] = '\0';