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

[Phil Yang <Phil.Yang@arm.com>]

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. 
http://patches.dpdk.org/patch/69382/

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?

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 
>