[PATCH] examples/cmdline: fix build error with gcc 12

[PATCH] examples/cmdline: fix build error with gcc 12

[Bruce Richardson]

When building the example without libbsd and using the DPDK-provided
strlcpy function, a compiler warning is emitted by GCC 12 about the copy
of the parsed string into the resulting object. This is because the
source from cmdline library is 128 bytes and the destination buffer is
64-bytes.

commands.c: In function 'cmd_obj_add_parsed':
.../__BUILDS/build-x86-generic/install/usr/local/include/rte_string_fns.h:61:24: warning: '%s' directive output may be truncated writing up to 127 bytes into a region of size 64 [-Wformat-truncation=]
   61 |         return (size_t)snprintf(dst, size, "%s", src);
      |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/stdio.h:894,
                 from commands.c:7:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:71:10: note: '__builtin_snprintf' output between 1 and 128 bytes into a destination of size 64

Multiple options are possible to fix this, but the one taken in this
patch is to ensure truncation never occurs by setting the destination
buffer size to be the same as that used by the cmdline library.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/cmdline/parse_obj_list.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/examples/cmdline/parse_obj_list.h b/examples/cmdline/parse_obj_list.h
index 6516d3e2c2..1223ac1e8b 100644
--- a/examples/cmdline/parse_obj_list.h
+++ b/examples/cmdline/parse_obj_list.h
@@ -12,8 +12,9 @@
 
 #include <sys/queue.h>
 #include <cmdline_parse.h>
+#include <cmdline_parse_string.h>
 
-#define OBJ_NAME_LEN_MAX 64
+#define OBJ_NAME_LEN_MAX sizeof(cmdline_fixed_string_t)
 
 struct object {
 	SLIST_ENTRY(object) next;
-- 
2.34.1

Re: [PATCH] examples/cmdline: fix build error with gcc 12

[Ferruh Yigit]

On 1/18/2023 4:11 PM, Bruce Richardson wrote:
> When building the example without libbsd and using the DPDK-provided
> strlcpy function, a compiler warning is emitted by GCC 12 about the copy
> of the parsed string into the resulting object. This is because the
> source from cmdline library is 128 bytes and the destination buffer is
> 64-bytes.
> 
> commands.c: In function 'cmd_obj_add_parsed':
> .../__BUILDS/build-x86-generic/install/usr/local/include/rte_string_fns.h:61:24: warning: '%s' directive output may be truncated writing up to 127 bytes into a region of size 64 [-Wformat-truncation=]
>    61 |         return (size_t)snprintf(dst, size, "%s", src);
>       |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from /usr/include/stdio.h:894,
>                  from commands.c:7:
> /usr/include/x86_64-linux-gnu/bits/stdio2.h:71:10: note: '__builtin_snprintf' output between 1 and 128 bytes into a destination of size 64
> 
> Multiple options are possible to fix this, but the one taken in this
> patch is to ensure truncation never occurs by setting the destination
> buffer size to be the same as that used by the cmdline library.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  examples/cmdline/parse_obj_list.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/examples/cmdline/parse_obj_list.h b/examples/cmdline/parse_obj_list.h
> index 6516d3e2c2..1223ac1e8b 100644
> --- a/examples/cmdline/parse_obj_list.h
> +++ b/examples/cmdline/parse_obj_list.h
> @@ -12,8 +12,9 @@
>  
>  #include <sys/queue.h>
>  #include <cmdline_parse.h>
> +#include <cmdline_parse_string.h>
>  
> -#define OBJ_NAME_LEN_MAX 64
> +#define OBJ_NAME_LEN_MAX sizeof(cmdline_fixed_string_t)
>  
>  struct object {
>  	SLIST_ENTRY(object) next;

I confirm it solves the build warning, but what about to get rid of
`OBJ_NAME_LEN_MAX` completely if the intentions is to make size same as
cmdline library array:

 diff --git a/examples/cmdline/parse_obj_list.c
b/examples/cmdline/parse_obj_list.c
 index 959bcd14527e..7b24bfb035d7 100644
 --- a/examples/cmdline/parse_obj_list.c
 +++ b/examples/cmdline/parse_obj_list.c
 @@ -46,7 +46,7 @@ parse_obj_list(cmdline_parse_token_hdr_t *tk, const
char *buf, void *res,
                 token_len++;

         SLIST_FOREACH(o, tkd->list, next) {
 -               if (token_len != strnlen(o->name, OBJ_NAME_LEN_MAX))
 +               if (token_len != strnlen(o->name, STR_TOKEN_SIZE))
                         continue;
                 if (strncmp(buf, o->name, token_len))
                         continue;
 @@ -91,7 +91,7 @@ int
complete_get_elt_obj_list(cmdline_parse_token_hdr_t *tk,
         if (!o)
                 return -1;

 -       len = strnlen(o->name, OBJ_NAME_LEN_MAX);
 +       len = strnlen(o->name, STR_TOKEN_SIZE);
         if ((len + 1) > size)
                 return -1;

 diff --git a/examples/cmdline/parse_obj_list.h
b/examples/cmdline/parse_obj_list.h
 index 6516d3e2c236..ba234601f106 100644
 --- a/examples/cmdline/parse_obj_list.h
 +++ b/examples/cmdline/parse_obj_list.h
 @@ -12,12 +12,11 @@

  #include <sys/queue.h>
  #include <cmdline_parse.h>
 -
 -#define OBJ_NAME_LEN_MAX 64
 +#include <cmdline_parse_string.h>

  struct object {
         SLIST_ENTRY(object) next;
 -       char name[OBJ_NAME_LEN_MAX];
 +       cmdline_fixed_string_t name;
         cmdline_ipaddr_t ip;
  };

Re: [PATCH] examples/cmdline: fix build error with gcc 12

[Bruce Richardson]

On Wed, Jan 18, 2023 at 06:53:33PM +0000, Ferruh Yigit wrote:
> On 1/18/2023 4:11 PM, Bruce Richardson wrote:
> > When building the example without libbsd and using the DPDK-provided
> > strlcpy function, a compiler warning is emitted by GCC 12 about the copy
> > of the parsed string into the resulting object. This is because the
> > source from cmdline library is 128 bytes and the destination buffer is
> > 64-bytes.
> > 
> > commands.c: In function 'cmd_obj_add_parsed':
> > .../__BUILDS/build-x86-generic/install/usr/local/include/rte_string_fns.h:61:24: warning: '%s' directive output may be truncated writing up to 127 bytes into a region of size 64 [-Wformat-truncation=]
> >    61 |         return (size_t)snprintf(dst, size, "%s", src);
> >       |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > In file included from /usr/include/stdio.h:894,
> >                  from commands.c:7:
> > /usr/include/x86_64-linux-gnu/bits/stdio2.h:71:10: note: '__builtin_snprintf' output between 1 and 128 bytes into a destination of size 64
> > 
> > Multiple options are possible to fix this, but the one taken in this
> > patch is to ensure truncation never occurs by setting the destination
> > buffer size to be the same as that used by the cmdline library.
> > 
> > Fixes: af75078fece3 ("first public release")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  examples/cmdline/parse_obj_list.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/examples/cmdline/parse_obj_list.h b/examples/cmdline/parse_obj_list.h
> > index 6516d3e2c2..1223ac1e8b 100644
> > --- a/examples/cmdline/parse_obj_list.h
> > +++ b/examples/cmdline/parse_obj_list.h
> > @@ -12,8 +12,9 @@
> >  
> >  #include <sys/queue.h>
> >  #include <cmdline_parse.h>
> > +#include <cmdline_parse_string.h>
> >  
> > -#define OBJ_NAME_LEN_MAX 64
> > +#define OBJ_NAME_LEN_MAX sizeof(cmdline_fixed_string_t)
> >  
> >  struct object {
> >  	SLIST_ENTRY(object) next;
> 
> I confirm it solves the build warning, but what about to get rid of
> `OBJ_NAME_LEN_MAX` completely if the intentions is to make size same as
> cmdline library array:
> 
Sure.
Another potential fix is just to cast-away [(void)] the return value from
strlcpy and allow truncation.

Olivier, as maintainer, what is your preferred fix here?

/Bruce

Re: [PATCH] examples/cmdline: fix build error with gcc 12

[Stephen Hemminger]

On Thu, 19 Jan 2023 08:59:10 +0000
Bruce Richardson <bruce.richardson@intel.com> wrote:

> > >  struct object {
> > >  	SLIST_ENTRY(object) next;  
> > 
> > I confirm it solves the build warning, but what about to get rid of
> > `OBJ_NAME_LEN_MAX` completely if the intentions is to make size same as
> > cmdline library array:
> >   
> Sure.
> Another potential fix is just to cast-away [(void)] the return value from
> strlcpy and allow truncation.
> 
> Olivier, as maintainer, what is your preferred fix here?
> 
> /Bruce

Another option would be to use flex-array and not have fixed size.

Re: [PATCH] examples/cmdline: fix build error with gcc 12

[Bruce Richardson]

On Thu, Jan 19, 2023 at 08:44:41AM -0800, Stephen Hemminger wrote:
> On Thu, 19 Jan 2023 08:59:10 +0000
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > > >  struct object {
> > > >  	SLIST_ENTRY(object) next;  
> > > 
> > > I confirm it solves the build warning, but what about to get rid of
> > > `OBJ_NAME_LEN_MAX` completely if the intentions is to make size same as
> > > cmdline library array:
> > >   
> > Sure.
> > Another potential fix is just to cast-away [(void)] the return value from
> > strlcpy and allow truncation.
> > 
> > Olivier, as maintainer, what is your preferred fix here?
> > 
> > /Bruce
> 
> Another option would be to use flex-array and not have fixed size.

I think that would add too many other additional complications. At least
using the static array makes the sizes clear and allows the compilers to
warn about possible truncation so we have to explicitly deal with it.

Re: [PATCH] examples/cmdline: fix build error with gcc 12

[Olivier Matz]

Hi,

Sorry for the late reply.

On Thu, Jan 19, 2023 at 08:59:10AM +0000, Bruce Richardson wrote:
> On Wed, Jan 18, 2023 at 06:53:33PM +0000, Ferruh Yigit wrote:
> > On 1/18/2023 4:11 PM, Bruce Richardson wrote:
> > > When building the example without libbsd and using the DPDK-provided
> > > strlcpy function, a compiler warning is emitted by GCC 12 about the copy
> > > of the parsed string into the resulting object. This is because the
> > > source from cmdline library is 128 bytes and the destination buffer is
> > > 64-bytes.
> > > 
> > > commands.c: In function 'cmd_obj_add_parsed':
> > > .../__BUILDS/build-x86-generic/install/usr/local/include/rte_string_fns.h:61:24: warning: '%s' directive output may be truncated writing up to 127 bytes into a region of size 64 [-Wformat-truncation=]
> > >    61 |         return (size_t)snprintf(dst, size, "%s", src);
> > >       |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > In file included from /usr/include/stdio.h:894,
> > >                  from commands.c:7:
> > > /usr/include/x86_64-linux-gnu/bits/stdio2.h:71:10: note: '__builtin_snprintf' output between 1 and 128 bytes into a destination of size 64
> > > 
> > > Multiple options are possible to fix this, but the one taken in this
> > > patch is to ensure truncation never occurs by setting the destination
> > > buffer size to be the same as that used by the cmdline library.
> > > 
> > > Fixes: af75078fece3 ("first public release")
> > > Cc: stable@dpdk.org
> > > 
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > ---
> > >  examples/cmdline/parse_obj_list.h | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/examples/cmdline/parse_obj_list.h b/examples/cmdline/parse_obj_list.h
> > > index 6516d3e2c2..1223ac1e8b 100644
> > > --- a/examples/cmdline/parse_obj_list.h
> > > +++ b/examples/cmdline/parse_obj_list.h
> > > @@ -12,8 +12,9 @@
> > >  
> > >  #include <sys/queue.h>
> > >  #include <cmdline_parse.h>
> > > +#include <cmdline_parse_string.h>
> > >  
> > > -#define OBJ_NAME_LEN_MAX 64
> > > +#define OBJ_NAME_LEN_MAX sizeof(cmdline_fixed_string_t)
> > >  
> > >  struct object {
> > >  	SLIST_ENTRY(object) next;
> > 
> > I confirm it solves the build warning, but what about to get rid of
> > `OBJ_NAME_LEN_MAX` completely if the intentions is to make size same as
> > cmdline library array:
> > 
> Sure.
> Another potential fix is just to cast-away [(void)] the return value from
> strlcpy and allow truncation.
> 
> Olivier, as maintainer, what is your preferred fix here?

I think you solution is good enough, given it's a dummy example.

Acked-by: Olivier Matz <olivier.matz@6wind.com>

Ferruh's comment about getting rid of OBJ_NAME_LEN_MAX is right, however
I think in this case we should allocate the string and store a pointer
to this string instead of having a size limited buffer.


Olivier

Re: [PATCH] examples/cmdline: fix build error with gcc 12

[Thomas Monjalon]

10/02/2023 12:26, Olivier Matz:
> Hi,
> 
> Sorry for the late reply.
> 
> On Thu, Jan 19, 2023 at 08:59:10AM +0000, Bruce Richardson wrote:
> > On Wed, Jan 18, 2023 at 06:53:33PM +0000, Ferruh Yigit wrote:
> > > On 1/18/2023 4:11 PM, Bruce Richardson wrote:
> > > > When building the example without libbsd and using the DPDK-provided
> > > > strlcpy function, a compiler warning is emitted by GCC 12 about the copy
> > > > of the parsed string into the resulting object. This is because the
> > > > source from cmdline library is 128 bytes and the destination buffer is
> > > > 64-bytes.
> > > > 
> > > > commands.c: In function 'cmd_obj_add_parsed':
> > > > .../__BUILDS/build-x86-generic/install/usr/local/include/rte_string_fns.h:61:24: warning: '%s' directive output may be truncated writing up to 127 bytes into a region of size 64 [-Wformat-truncation=]
> > > >    61 |         return (size_t)snprintf(dst, size, "%s", src);
> > > >       |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > In file included from /usr/include/stdio.h:894,
> > > >                  from commands.c:7:
> > > > /usr/include/x86_64-linux-gnu/bits/stdio2.h:71:10: note: '__builtin_snprintf' output between 1 and 128 bytes into a destination of size 64
> > > > 
> > > > Multiple options are possible to fix this, but the one taken in this
> > > > patch is to ensure truncation never occurs by setting the destination
> > > > buffer size to be the same as that used by the cmdline library.
> > > > 
> > > > Fixes: af75078fece3 ("first public release")
> > > > Cc: stable@dpdk.org
> > > > 
> > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > ---
> > > >  examples/cmdline/parse_obj_list.h | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/examples/cmdline/parse_obj_list.h b/examples/cmdline/parse_obj_list.h
> > > > index 6516d3e2c2..1223ac1e8b 100644
> > > > --- a/examples/cmdline/parse_obj_list.h
> > > > +++ b/examples/cmdline/parse_obj_list.h
> > > > @@ -12,8 +12,9 @@
> > > >  
> > > >  #include <sys/queue.h>
> > > >  #include <cmdline_parse.h>
> > > > +#include <cmdline_parse_string.h>
> > > >  
> > > > -#define OBJ_NAME_LEN_MAX 64
> > > > +#define OBJ_NAME_LEN_MAX sizeof(cmdline_fixed_string_t)
> > > >  
> > > >  struct object {
> > > >  	SLIST_ENTRY(object) next;
> > > 
> > > I confirm it solves the build warning, but what about to get rid of
> > > `OBJ_NAME_LEN_MAX` completely if the intentions is to make size same as
> > > cmdline library array:
> > > 
> > Sure.
> > Another potential fix is just to cast-away [(void)] the return value from
> > strlcpy and allow truncation.
> > 
> > Olivier, as maintainer, what is your preferred fix here?
> 
> I think you solution is good enough, given it's a dummy example.
> 
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
> 
> Ferruh's comment about getting rid of OBJ_NAME_LEN_MAX is right, however
> I think in this case we should allocate the string and store a pointer
> to this string instead of having a size limited buffer.

Applied this simple patch.