examples/cmdline: fix build error with gcc 12

Message ID 20230118161111.11710-1-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series examples/cmdline: fix build error with gcc 12 |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Performance fail Performance Testing issues
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/github-robot: build success github build: passed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-testing fail Testing issues
ci/iol-x86_64-unit-testing fail Testing issues
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Bruce Richardson Jan. 18, 2023, 4:11 p.m. UTC
  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(-)
  

Comments

Ferruh Yigit Jan. 18, 2023, 6:53 p.m. UTC | #1
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;
  };
  
Bruce Richardson Jan. 19, 2023, 8:59 a.m. UTC | #2
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
  
Stephen Hemminger Jan. 19, 2023, 4:44 p.m. UTC | #3
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.
  
Bruce Richardson Jan. 19, 2023, 6:12 p.m. UTC | #4
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.
  
Olivier Matz Feb. 10, 2023, 11:26 a.m. UTC | #5
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
  
Thomas Monjalon Feb. 20, 2023, 11:44 a.m. UTC | #6
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.
  

Patch

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;