Re: [dpdk-dev] [EXT] [PATCH] trace: fix build with gcc 10

[Sunil Kumar Kori <skori@marvell.com>]

>-----Original Message-----
>From: Phil Yang <Phil.Yang@arm.com>
>Sent: Tuesday, April 28, 2020 12:30 PM
>To: Sunil Kumar Kori <skori@marvell.com>; Jerin Jacob Kollanukkaran
><jerinj@marvell.com>; dev@dpdk.org
>Cc: David Marchand <david.marchand@redhat.com>; Ruifeng Wang
><Ruifeng.Wang@arm.com>; Lijian Zhang <Lijian.Zhang@arm.com>; nd
><nd@arm.com>; nd <nd@arm.com>
>Subject: RE: [EXT] [PATCH] trace: fix build with gcc 10
>
>From: Sunil Kumar Kori <skori@marvell.com>
>Sent: Tuesday, April 28, 2020 11:49 AM
>To: Phil Yang <Phil.Yang@arm.com>; jerinj@marvell.com; dev@dpdk.org
>Cc: David Marchand <david.marchand@redhat.com>; Ruifeng Wang
><Ruifeng.Wang@arm.com>; Lijian Zhang <Lijian.Zhang@arm.com>; nd
><nd@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.yang@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.yang@arm.com>
>> Reviewed-by: Lijian Zhang <mailto:lijian.zhang@arm.com>
>> Tested-by: Lijian Zhang <mailto:lijian.zhang@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
>>