>-----Original Message----- >From: Phil Yang <phil.y...@arm.com> >Sent: Tuesday, April 28, 2020 12:30 PM >To: Sunil Kumar Kori <sk...@marvell.com>; Jerin Jacob Kollanukkaran ><jer...@marvell.com>; dev@dpdk.org >Cc: David Marchand <david.march...@redhat.com>; Ruifeng Wang ><ruifeng.w...@arm.com>; Lijian Zhang <lijian.zh...@arm.com>; nd ><n...@arm.com>; nd <n...@arm.com> >Subject: RE: [EXT] [PATCH] trace: fix build with gcc 10 > >From: Sunil Kumar Kori <sk...@marvell.com> >Sent: Tuesday, April 28, 2020 11:49 AM >To: Phil Yang <phil.y...@arm.com>; jer...@marvell.com; dev@dpdk.org >Cc: David Marchand <david.march...@redhat.com>; Ruifeng Wang ><ruifeng.w...@arm.com>; Lijian Zhang <lijian.zh...@arm.com>; nd ><n...@arm.com> >Subject: [EXT] [PATCH] trace: fix build with gcc 10 > >Sent from Workspace ONE Boxer >On 27-Apr-2020 10:18 PM, Phil Yang <mailto:phil.y...@arm.com> wrote: >> >> External Email >> >> ---------------------------------------------------------------------- >> GCC 10 compiling output: >> eal_common_trace_utils.c: In function 'eal_trace_dir_args_save': >> eal_common_trace_utils.c:290:24: error: '__builtin___sprintf_chk' \ >> may write a terminating nul past the end of the destination \ >> [-Werror=format-overflow=] >> 290 | sprintf(dir_path, "%s/", optarg); >> | ^ >> >> Fixes: 8af866df8d8c ("trace: add trace directory configuration parameter") >> >Hello, there is one more thread going on regarding this. Please have a look on >below patch. >https://urldefense.proofpoint.com/v2/url?u=http- >3A__patches.dpdk.org_patch_69382_&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7x >tfQ&r=dXeXaAMkP5COgn1zxHMyaF1_d9IIuq6vHQO6NrIPjaE&m=WFCcD0EavY >uGMZUUoJWIQunwTAwgxAju2rK4s3Nr-t4&s=iMF8PSGMB8S- >rDR0kJGOZ1el3MzeOKfxZQxX-Oyg54g&e= > >Hi Sunil, > >Sorry, I didn’t notice that. Thanks for the link. > >I have two points: >1. Will this patch resolves both mentioned warnings/error in patch 69382 ? >[Phil] Yes, this patch resolved the same issue mentioned by David in patch >69382. > >2. David has suggested another way of doing it. Please check that too. >[Phil] I think both David’s and my patches are correct. >My patch can guarantee a correct ‘size’ information in snprinf(). It omits the >memory allocation operation for the incorrect input arguments case. >David’s suggestion resolves the potential directory copy fail issue and it >saves >some memory space in the normal case. But it needs to allocate memory in >the incorrect input case. > >So, I think we can bind these two patches together? Make sense. So can you please combine both the patches and share ?
> >Thanks, >Phil > >> Signed-off-by: Phil Yang <mailto:phil.y...@arm.com> >> Reviewed-by: Lijian Zhang <mailto:lijian.zh...@arm.com> >> Tested-by: Lijian Zhang <mailto:lijian.zh...@arm.com> >> --- >> lib/librte_eal/common/eal_common_trace_utils.c | 5 ++++- >> 1 file changed, 4 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 fce8892..c079642 100644 >> --- a/lib/librte_eal/common/eal_common_trace_utils.c >> +++ b/lib/librte_eal/common/eal_common_trace_utils.c >> @@ -276,7 +276,10 @@ eal_trace_dir_args_save(char const *optarg) >> return -EINVAL; >> } >> >> - if (strlen(optarg) >= size) { >> + /* the specified trace directory name cannot >> + * exceed PATH_MAX-1. >> + */ >> + if (strlen(optarg) >= (size - 1)) { >> trace_err("input string is too big"); >> return -ENAMETOOLONG; >> } >> -- >> 2.7.4 >>
