Pointer was being dereferenced without NULL checking. Coverity issue: 357768 Fixes: 8c8066ea6a7b ("trace: add trace mode configuration parameter") Signed-off-by: Sunil Kumar Kori <skori@marvell.com> --- lib/librte_eal/common/eal_common_trace_utils.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/librte_eal/common/eal_common_trace_utils.c b/lib/librte_eal/common/eal_common_trace_utils.c index fce8892c3..119e97119 100644 --- a/lib/librte_eal/common/eal_common_trace_utils.c +++ b/lib/librte_eal/common/eal_common_trace_utils.c @@ -227,15 +227,16 @@ int eal_trace_mode_args_save(const char *optarg) { struct trace *trace = trace_obj_get(); - size_t len = strlen(optarg); unsigned long tmp; char *pattern; + size_t len; if (optarg == NULL) { trace_err("no optarg is passed"); return -EINVAL; } + len = strlen(optarg); if (len == 0) { trace_err("value is not provided with option"); return -EINVAL; -- 2.17.1
On Mon, Apr 27, 2020 at 2:04 PM Sunil Kumar Kori <skori@marvell.com> wrote: > > Pointer was being dereferenced without NULL checking. > > Coverity issue: 357768 > > Fixes: 8c8066ea6a7b ("trace: add trace mode configuration parameter") > > Signed-off-by: Sunil Kumar Kori <skori@marvell.com> > --- > lib/librte_eal/common/eal_common_trace_utils.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_eal/common/eal_common_trace_utils.c b/lib/librte_eal/common/eal_common_trace_utils.c > index fce8892c3..119e97119 100644 > --- a/lib/librte_eal/common/eal_common_trace_utils.c > +++ b/lib/librte_eal/common/eal_common_trace_utils.c > @@ -227,15 +227,16 @@ int > eal_trace_mode_args_save(const char *optarg) > { > struct trace *trace = trace_obj_get(); > - size_t len = strlen(optarg); > unsigned long tmp; > char *pattern; > + size_t len; > > if (optarg == NULL) { > trace_err("no optarg is passed"); > return -EINVAL; > } > > + len = strlen(optarg); > if (len == 0) { > trace_err("value is not provided with option"); > return -EINVAL; I was looking at some gcc 10 complaints on string manipulation later in eal_trace_dir_args_save(). https://build.opensuse.org/package/live_build_log/home:dmarchan:branches:home:bluca:dpdk/dpdk/Fedora_Rawhide/x86_64 [ 126s] CC rte_malloc.o [ 127s] /home/abuild/rpmbuild/BUILD/dpdk-1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c: In function 'eal_trace_dir_args_save': [ 127s] /home/abuild/rpmbuild/BUILD/dpdk-1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c:290:24: error: 'sprintf' may write a terminating nul past the end of the destination [-Werror=format-overflow=] [ 127s] 290 | sprintf(dir_path, "%s/", optarg); [ 127s] | ^ [ 127s] /home/abuild/rpmbuild/BUILD/dpdk-1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c:290:2: note: 'sprintf' output between 2 and 4097 bytes into a destination of size 4096 [ 127s] 290 | sprintf(dir_path, "%s/", optarg); [ 127s] | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [ 127s] cc1: all warnings being treated as errors Could we use asprintf in all this code and avoid malloc + sprintf ? Something like the _untested_ snippet? diff --git a/lib/librte_eal/common/eal_common_trace_utils.c b/lib/librte_eal/common/eal_common_trace_utils.c index fce8892c38..4769bade97 100644 --- a/lib/librte_eal/common/eal_common_trace_utils.c +++ b/lib/librte_eal/common/eal_common_trace_utils.c @@ -267,8 +267,7 @@ int eal_trace_dir_args_save(char const *optarg) { struct trace *trace = trace_obj_get(); - uint32_t size = sizeof(trace->dir); - char *dir_path = NULL; + char *dir_path; int rc; if (optarg == NULL) { @@ -276,19 +275,18 @@ eal_trace_dir_args_save(char const *optarg) return -EINVAL; } - if (strlen(optarg) >= size) { - trace_err("input string is too big"); - return -ENAMETOOLONG; - } - - dir_path = (char *)calloc(1, size); - if (dir_path == NULL) { - trace_err("fail to allocate memory"); + rc = asprintf(&dir_path, "%s/", optarg); + if (rc == -1) { + trace_err("failed to copy directory: %s", strerror(errno)); return -ENOMEM; } - sprintf(dir_path, "%s/", optarg); - rc = trace_dir_update(dir_path); + if ((size_t)rc >= sizeof(trace->dir)) { + trace_err("input string is too big"); + rc = -ENAMETOOLONG; + } else { + rc = trace_dir_update(dir_path); + } free(dir_path); return rc; -- David Marchand
>-----Original Message----- >From: David Marchand <david.marchand@redhat.com> >Sent: Monday, April 27, 2020 5:56 PM >To: Sunil Kumar Kori <skori@marvell.com> >Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; dev <dev@dpdk.org> >Subject: [EXT] Re: [dpdk-dev] [PATCH] eal/trace: fix coverity issues > >External Email > >---------------------------------------------------------------------- >On Mon, Apr 27, 2020 at 2:04 PM Sunil Kumar Kori <skori@marvell.com> >wrote: >> >> Pointer was being dereferenced without NULL checking. >> >> Coverity issue: 357768 >> >> Fixes: 8c8066ea6a7b ("trace: add trace mode configuration parameter") >> >> Signed-off-by: Sunil Kumar Kori <skori@marvell.com> >> --- >> lib/librte_eal/common/eal_common_trace_utils.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/lib/librte_eal/common/eal_common_trace_utils.c >> b/lib/librte_eal/common/eal_common_trace_utils.c >> index fce8892c3..119e97119 100644 >> --- a/lib/librte_eal/common/eal_common_trace_utils.c >> +++ b/lib/librte_eal/common/eal_common_trace_utils.c >> @@ -227,15 +227,16 @@ int >> eal_trace_mode_args_save(const char *optarg) { >> struct trace *trace = trace_obj_get(); >> - size_t len = strlen(optarg); >> unsigned long tmp; >> char *pattern; >> + size_t len; >> >> if (optarg == NULL) { >> trace_err("no optarg is passed"); >> return -EINVAL; >> } >> >> + len = strlen(optarg); >> if (len == 0) { >> trace_err("value is not provided with option"); >> return -EINVAL; > >I was looking at some gcc 10 complaints on string manipulation later >in eal_trace_dir_args_save(). > >https://urldefense.proofpoint.com/v2/url?u=https- >3A__build.opensuse.org_package_live-5Fbuild-5Flog_home-3Admarchan- >3Abranches-3Ahome-3Abluca-3Adpdk_dpdk_Fedora-5FRawhide_x86- >5F64&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=dXeXaAMkP5COgn1zxHMy >aF1_d9IIuq6vHQO6NrIPjaE&m=NZ72Sr2OMEYZD7PIY59lshlAxZJJJepF5oxbHv0j >5Zg&s=yOCA3PfhZojqJv0iVKlzeqM7tYGVv0jjrnVcajUx_qA&e= > >[ 126s] CC rte_malloc.o >[ 127s] /home/abuild/rpmbuild/BUILD/dpdk- >1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c: >In function 'eal_trace_dir_args_save': >[ 127s] /home/abuild/rpmbuild/BUILD/dpdk- >1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c:29 >0:24: >error: 'sprintf' may write a terminating nul past the end of the >destination [-Werror=format-overflow=] >[ 127s] 290 | sprintf(dir_path, "%s/", optarg); >[ 127s] | ^ >[ 127s] /home/abuild/rpmbuild/BUILD/dpdk- >1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c:29 >0:2: >note: 'sprintf' output between 2 and 4097 bytes into a destination of >size 4096 >[ 127s] 290 | sprintf(dir_path, "%s/", optarg); >[ 127s] | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >[ 127s] cc1: all warnings being treated as errors > > >Could we use asprintf in all this code and avoid malloc + sprintf ? > As I understood from above warnings/errors, real problem is writing beyond destination i.e. dir_path. If this is the case then it can be simply handled using snprintf(); with correct "size" information. Suggested code changes are correct. I am just trying to achieve this with lesser code changes. Also I think, fix for this should be a separate patch. Suggestions please ? > >Something like the _untested_ snippet? > >diff --git a/lib/librte_eal/common/eal_common_trace_utils.c >b/lib/librte_eal/common/eal_common_trace_utils.c >index fce8892c38..4769bade97 100644 >--- a/lib/librte_eal/common/eal_common_trace_utils.c >+++ b/lib/librte_eal/common/eal_common_trace_utils.c >@@ -267,8 +267,7 @@ int > eal_trace_dir_args_save(char const *optarg) > { > struct trace *trace = trace_obj_get(); >- uint32_t size = sizeof(trace->dir); >- char *dir_path = NULL; >+ char *dir_path; > int rc; > > if (optarg == NULL) { >@@ -276,19 +275,18 @@ eal_trace_dir_args_save(char const *optarg) > return -EINVAL; > } > >- if (strlen(optarg) >= size) { >- trace_err("input string is too big"); >- return -ENAMETOOLONG; >- } >- >- dir_path = (char *)calloc(1, size); >- if (dir_path == NULL) { >- trace_err("fail to allocate memory"); >+ rc = asprintf(&dir_path, "%s/", optarg); >+ if (rc == -1) { >+ trace_err("failed to copy directory: %s", strerror(errno)); > return -ENOMEM; > } > >- sprintf(dir_path, "%s/", optarg); >- rc = trace_dir_update(dir_path); >+ if ((size_t)rc >= sizeof(trace->dir)) { >+ trace_err("input string is too big"); >+ rc = -ENAMETOOLONG; >+ } else { >+ rc = trace_dir_update(dir_path); >+ } > > free(dir_path); > return rc; > > > >-- >David Marchand
On Mon, Apr 27, 2020 at 3:46 PM Sunil Kumar Kori <skori@marvell.com> wrote:
>
> >-----Original Message-----
> >From: David Marchand <david.marchand@redhat.com>
> >Sent: Monday, April 27, 2020 5:56 PM
> >To: Sunil Kumar Kori <skori@marvell.com>
> >Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; dev <dev@dpdk.org>
> >Subject: [EXT] Re: [dpdk-dev] [PATCH] eal/trace: fix coverity issues
> >
> >External Email
> >
> >----------------------------------------------------------------------
> >On Mon, Apr 27, 2020 at 2:04 PM Sunil Kumar Kori <skori@marvell.com>
> >wrote:
> >>
> >> Pointer was being dereferenced without NULL checking.
> >>
> >> Coverity issue: 357768
> >>
> >> Fixes: 8c8066ea6a7b ("trace: add trace mode configuration parameter")
> >>
> >> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> >> ---
> >> lib/librte_eal/common/eal_common_trace_utils.c | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/lib/librte_eal/common/eal_common_trace_utils.c
> >> b/lib/librte_eal/common/eal_common_trace_utils.c
> >> index fce8892c3..119e97119 100644
> >> --- a/lib/librte_eal/common/eal_common_trace_utils.c
> >> +++ b/lib/librte_eal/common/eal_common_trace_utils.c
> >> @@ -227,15 +227,16 @@ int
> >> eal_trace_mode_args_save(const char *optarg) {
> >> struct trace *trace = trace_obj_get();
> >> - size_t len = strlen(optarg);
> >> unsigned long tmp;
> >> char *pattern;
> >> + size_t len;
> >>
> >> if (optarg == NULL) {
> >> trace_err("no optarg is passed");
> >> return -EINVAL;
> >> }
> >>
> >> + len = strlen(optarg);
> >> if (len == 0) {
> >> trace_err("value is not provided with option");
> >> return -EINVAL;
> >
> >I was looking at some gcc 10 complaints on string manipulation later
> >in eal_trace_dir_args_save().
> >
> >https://urldefense.proofpoint.com/v2/url?u=https-
> >3A__build.opensuse.org_package_live-5Fbuild-5Flog_home-3Admarchan-
> >3Abranches-3Ahome-3Abluca-3Adpdk_dpdk_Fedora-5FRawhide_x86-
> >5F64&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=dXeXaAMkP5COgn1zxHMy
> >aF1_d9IIuq6vHQO6NrIPjaE&m=NZ72Sr2OMEYZD7PIY59lshlAxZJJJepF5oxbHv0j
> >5Zg&s=yOCA3PfhZojqJv0iVKlzeqM7tYGVv0jjrnVcajUx_qA&e=
> >
> >[ 126s] CC rte_malloc.o
> >[ 127s] /home/abuild/rpmbuild/BUILD/dpdk-
> >1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c:
> >In function 'eal_trace_dir_args_save':
> >[ 127s] /home/abuild/rpmbuild/BUILD/dpdk-
> >1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c:29
> >0:24:
> >error: 'sprintf' may write a terminating nul past the end of the
> >destination [-Werror=format-overflow=]
> >[ 127s] 290 | sprintf(dir_path, "%s/", optarg);
> >[ 127s] | ^
> >[ 127s] /home/abuild/rpmbuild/BUILD/dpdk-
> >1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c:29
> >0:2:
> >note: 'sprintf' output between 2 and 4097 bytes into a destination of
> >size 4096
> >[ 127s] 290 | sprintf(dir_path, "%s/", optarg);
> >[ 127s] | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >[ 127s] cc1: all warnings being treated as errors
> >
> >
> >Could we use asprintf in all this code and avoid malloc + sprintf ?
> >
> As I understood from above warnings/errors, real problem is writing beyond destination i.e. dir_path.
> If this is the case then it can be simply handled using snprintf(); with correct "size" information.
> Suggested code changes are correct. I am just trying to achieve this with lesser code changes.
>
> Also I think, fix for this should be a separate patch.
> Suggestions please ?
If the code was not combining this malloc + sprintf + wrong checks
(caught by coverity), we would avoid both issues and it would be more
consistent.
No more suggestion from me.
--
David Marchand
>-----Original Message----- >From: David Marchand <david.marchand@redhat.com> >Sent: Monday, April 27, 2020 7:22 PM >To: Sunil Kumar Kori <skori@marvell.com> >Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; dev <dev@dpdk.org> >Subject: Re: [EXT] Re: [dpdk-dev] [PATCH] eal/trace: fix coverity issues > >On Mon, Apr 27, 2020 at 3:46 PM Sunil Kumar Kori <skori@marvell.com> >wrote: >> >> >-----Original Message----- >> >From: David Marchand <david.marchand@redhat.com> >> >Sent: Monday, April 27, 2020 5:56 PM >> >To: Sunil Kumar Kori <skori@marvell.com> >> >Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; dev >> ><dev@dpdk.org> >> >Subject: [EXT] Re: [dpdk-dev] [PATCH] eal/trace: fix coverity issues >> > >> >External Email >> > >> >--------------------------------------------------------------------- >> >- On Mon, Apr 27, 2020 at 2:04 PM Sunil Kumar Kori >> ><skori@marvell.com> >> >wrote: >> >> >> >> Pointer was being dereferenced without NULL checking. >> >> >> >> Coverity issue: 357768 >> >> >> >> Fixes: 8c8066ea6a7b ("trace: add trace mode configuration >> >> parameter") >> >> >> >> Signed-off-by: Sunil Kumar Kori <skori@marvell.com> >> >> --- >> >> lib/librte_eal/common/eal_common_trace_utils.c | 3 ++- >> >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/lib/librte_eal/common/eal_common_trace_utils.c >> >> b/lib/librte_eal/common/eal_common_trace_utils.c >> >> index fce8892c3..119e97119 100644 >> >> --- a/lib/librte_eal/common/eal_common_trace_utils.c >> >> +++ b/lib/librte_eal/common/eal_common_trace_utils.c >> >> @@ -227,15 +227,16 @@ int >> >> eal_trace_mode_args_save(const char *optarg) { >> >> struct trace *trace = trace_obj_get(); >> >> - size_t len = strlen(optarg); >> >> unsigned long tmp; >> >> char *pattern; >> >> + size_t len; >> >> >> >> if (optarg == NULL) { >> >> trace_err("no optarg is passed"); >> >> return -EINVAL; >> >> } >> >> >> >> + len = strlen(optarg); >> >> if (len == 0) { >> >> trace_err("value is not provided with option"); >> >> return -EINVAL; >> > >> >I was looking at some gcc 10 complaints on string manipulation later >> >in eal_trace_dir_args_save(). >> > >> >https://urldefense.proofpoint.com/v2/url?u=https- >> >3A__build.opensuse.org_package_live-5Fbuild-5Flog_home-3Admarchan- >> >3Abranches-3Ahome-3Abluca-3Adpdk_dpdk_Fedora-5FRawhide_x86- >> >>5F64&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=dXeXaAMkP5COgn1zxHM >y >> >>aF1_d9IIuq6vHQO6NrIPjaE&m=NZ72Sr2OMEYZD7PIY59lshlAxZJJJepF5oxbHv0 >j >> >5Zg&s=yOCA3PfhZojqJv0iVKlzeqM7tYGVv0jjrnVcajUx_qA&e= >> > >> >[ 126s] CC rte_malloc.o >> >[ 127s] /home/abuild/rpmbuild/BUILD/dpdk- >> >1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c: >> >In function 'eal_trace_dir_args_save': >> >[ 127s] /home/abuild/rpmbuild/BUILD/dpdk- >> >>1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c:2 >> >9 >> >0:24: >> >error: 'sprintf' may write a terminating nul past the end of the >> >destination [-Werror=format-overflow=] >> >[ 127s] 290 | sprintf(dir_path, "%s/", optarg); >> >[ 127s] | ^ >> >[ 127s] /home/abuild/rpmbuild/BUILD/dpdk- >> >>1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c:2 >> >9 >> >0:2: >> >note: 'sprintf' output between 2 and 4097 bytes into a destination of >> >size 4096 >> >[ 127s] 290 | sprintf(dir_path, "%s/", optarg); >> >[ 127s] | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >[ 127s] cc1: all warnings being treated as errors >> > >> > >> >Could we use asprintf in all this code and avoid malloc + sprintf ? >> > >> As I understood from above warnings/errors, real problem is writing beyond >destination i.e. dir_path. >> If this is the case then it can be simply handled using snprintf(); with correct >"size" information. >> Suggested code changes are correct. I am just trying to achieve this with >lesser code changes. >> >> Also I think, fix for this should be a separate patch. >> Suggestions please ? > >If the code was not combining this malloc + sprintf + wrong checks (caught by >coverity), we would avoid both issues and it would be more consistent. >No more suggestion from me. Agreed but It must be a separate patch because there could be other warnings too thrown by GCC 10.x which may require handling. > > >-- >David Marchand
Hello David, Mentioned patch (http://patches.dpdk.org/patch/69467/ ) takes care of your input to resolve GCC 10 build. Now I think, this patch is good to go. Please take care of this. Regards Sunil Kumar Kori >-----Original Message----- >From: David Marchand <david.marchand@redhat.com> >Sent: Monday, April 27, 2020 7:22 PM >To: Sunil Kumar Kori <skori@marvell.com> >Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; dev <dev@dpdk.org> >Subject: Re: [EXT] Re: [dpdk-dev] [PATCH] eal/trace: fix coverity issues > >On Mon, Apr 27, 2020 at 3:46 PM Sunil Kumar Kori <skori@marvell.com> >wrote: >> >> >-----Original Message----- >> >From: David Marchand <david.marchand@redhat.com> >> >Sent: Monday, April 27, 2020 5:56 PM >> >To: Sunil Kumar Kori <skori@marvell.com> >> >Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; dev >> ><dev@dpdk.org> >> >Subject: [EXT] Re: [dpdk-dev] [PATCH] eal/trace: fix coverity issues >> > >> >External Email >> > >> >--------------------------------------------------------------------- >> >- On Mon, Apr 27, 2020 at 2:04 PM Sunil Kumar Kori >> ><skori@marvell.com> >> >wrote: >> >> >> >> Pointer was being dereferenced without NULL checking. >> >> >> >> Coverity issue: 357768 >> >> >> >> Fixes: 8c8066ea6a7b ("trace: add trace mode configuration >> >> parameter") >> >> >> >> Signed-off-by: Sunil Kumar Kori <skori@marvell.com> >> >> --- >> >> lib/librte_eal/common/eal_common_trace_utils.c | 3 ++- >> >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/lib/librte_eal/common/eal_common_trace_utils.c >> >> b/lib/librte_eal/common/eal_common_trace_utils.c >> >> index fce8892c3..119e97119 100644 >> >> --- a/lib/librte_eal/common/eal_common_trace_utils.c >> >> +++ b/lib/librte_eal/common/eal_common_trace_utils.c >> >> @@ -227,15 +227,16 @@ int >> >> eal_trace_mode_args_save(const char *optarg) { >> >> struct trace *trace = trace_obj_get(); >> >> - size_t len = strlen(optarg); >> >> unsigned long tmp; >> >> char *pattern; >> >> + size_t len; >> >> >> >> if (optarg == NULL) { >> >> trace_err("no optarg is passed"); >> >> return -EINVAL; >> >> } >> >> >> >> + len = strlen(optarg); >> >> if (len == 0) { >> >> trace_err("value is not provided with option"); >> >> return -EINVAL; >> > >> >I was looking at some gcc 10 complaints on string manipulation later >> >in eal_trace_dir_args_save(). >> > >> >https://urldefense.proofpoint.com/v2/url?u=https- >> >3A__build.opensuse.org_package_live-5Fbuild-5Flog_home-3Admarchan- >> >3Abranches-3Ahome-3Abluca-3Adpdk_dpdk_Fedora-5FRawhide_x86- >> >>5F64&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=dXeXaAMkP5COgn1zxHM >y >> >>aF1_d9IIuq6vHQO6NrIPjaE&m=NZ72Sr2OMEYZD7PIY59lshlAxZJJJepF5oxbHv0 >j >> >5Zg&s=yOCA3PfhZojqJv0iVKlzeqM7tYGVv0jjrnVcajUx_qA&e= >> > >> >[ 126s] CC rte_malloc.o >> >[ 127s] /home/abuild/rpmbuild/BUILD/dpdk- >> >1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c: >> >In function 'eal_trace_dir_args_save': >> >[ 127s] /home/abuild/rpmbuild/BUILD/dpdk- >> >>1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c:2 >> >9 >> >0:24: >> >error: 'sprintf' may write a terminating nul past the end of the >> >destination [-Werror=format-overflow=] >> >[ 127s] 290 | sprintf(dir_path, "%s/", optarg); >> >[ 127s] | ^ >> >[ 127s] /home/abuild/rpmbuild/BUILD/dpdk- >> >>1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c:2 >> >9 >> >0:2: >> >note: 'sprintf' output between 2 and 4097 bytes into a destination of >> >size 4096 >> >[ 127s] 290 | sprintf(dir_path, "%s/", optarg); >> >[ 127s] | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >[ 127s] cc1: all warnings being treated as errors >> > >> > >> >Could we use asprintf in all this code and avoid malloc + sprintf ? >> > >> As I understood from above warnings/errors, real problem is writing beyond >destination i.e. dir_path. >> If this is the case then it can be simply handled using snprintf(); with correct >"size" information. >> Suggested code changes are correct. I am just trying to achieve this with >lesser code changes. >> >> Also I think, fix for this should be a separate patch. >> Suggestions please ? > >If the code was not combining this malloc + sprintf + wrong checks (caught by >coverity), we would avoid both issues and it would be more consistent. >No more suggestion from me. > > >-- >David Marchand