eal: fix log message print for regex

Message ID 20200227082506.25349-1-skori@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series eal: fix log message print for regex |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-testing success Testing PASS
ci/travis-robot success Travis build: passed

Commit Message

Sunil Kumar Kori Feb. 27, 2020, 8:25 a.m. UTC
  If user passes log-level eal parameter to enable log level based on regex
then in case of error message is being printed for pattern match instead of
regex. Following is the warning message thrown:

Compiling C object 'lib/76b5a35@@rte_eal@sta/librte_eal_common_eal_common_options.c.o'.
In function ‘eal_parse_log_level’,
   inlined from ‘eal_parse_common_option’ at ../lib/librte_eal/common/eal_common_options.c:1418:7:
../lib/librte_eal/common/eal_common_options.c:1053:4: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
 1053 |    fprintf(stderr, "cannot set log level %s,%d\n",
      |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1054 |     pattern, priority);
      |     ~~~~~~~~~~~~~~~~~~

Fixes: 7f0bb634a140 ("log: add ability to match log type with globbing")

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
 lib/librte_eal/common/eal_common_options.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

David Marchand Feb. 27, 2020, 10:57 a.m. UTC | #1
On Thu, Feb 27, 2020 at 9:25 AM Sunil Kumar Kori <skori@marvell.com> wrote:
>
> If user passes log-level eal parameter to enable log level based on regex
> then in case of error message is being printed for pattern match instead of
> regex. Following is the warning message thrown:
>
> Compiling C object 'lib/76b5a35@@rte_eal@sta/librte_eal_common_eal_common_options.c.o'.
> In function ‘eal_parse_log_level’,
>    inlined from ‘eal_parse_common_option’ at ../lib/librte_eal/common/eal_common_options.c:1418:7:
> ../lib/librte_eal/common/eal_common_options.c:1053:4: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
>  1053 |    fprintf(stderr, "cannot set log level %s,%d\n",
>       |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  1054 |     pattern, priority);
>       |     ~~~~~~~~~~~~~~~~~~
>

Good catch.

Just to understand, how did you catch it?
Extra cflags? specific compiler?


> Fixes: 7f0bb634a140 ("log: add ability to match log type with globbing")

Missing Cc: stable.


>
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> ---
>  lib/librte_eal/common/eal_common_options.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index 75974dd5b..525e51e7d 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -1046,7 +1046,7 @@ eal_parse_log_level(const char *arg)
>         if (regex) {
>                 if (rte_log_set_level_regexp(regex, priority) < 0) {
>                         fprintf(stderr, "cannot set log level %s,%d\n",
> -                               pattern, priority);
> +                               regex, priority);
>                         goto fail;
>                 }
>                 if (rte_log_save_regexp(regex, priority) < 0)
> --
> 2.17.1
>

Acked-by: David Marchand <david.marchand@redhat.com>



--
David Marchand
  
Sunil Kumar Kori Feb. 27, 2020, 11:02 a.m. UTC | #2
Hello David,

We have two build machine with two different version of GCC. 
So the issue is reported on one setup only not on the other one.
It looks like because of different version of GCC, this warning is caught. 

Regards
Sunil Kumar Kori

>-----Original Message-----
>From: David Marchand <david.marchand@redhat.com>
>Sent: Thursday, February 27, 2020 4:28 PM
>To: Sunil Kumar Kori <skori@marvell.com>
>Cc: Olivier Matz <olivier.matz@6wind.com>; Thomas Monjalon
><thomas@monjalon.net>; dev <dev@dpdk.org>; dpdk stable
><stable@dpdk.org>
>Subject: [EXT] Re: [dpdk-stable] [PATCH] eal: fix log message print for regex
>
>External Email
>
>----------------------------------------------------------------------
>On Thu, Feb 27, 2020 at 9:25 AM Sunil Kumar Kori <skori@marvell.com>
>wrote:
>>
>> If user passes log-level eal parameter to enable log level based on
>> regex then in case of error message is being printed for pattern match
>> instead of regex. Following is the warning message thrown:
>>
>> Compiling C object
>'lib/76b5a35@@rte_eal@sta/librte_eal_common_eal_common_options.c.o'.
>> In function ‘eal_parse_log_level’,
>>    inlined from ‘eal_parse_common_option’ at
>../lib/librte_eal/common/eal_common_options.c:1418:7:
>> ../lib/librte_eal/common/eal_common_options.c:1053:4: warning: ‘%s’
>directive argument is null [-Wformat-overflow=]
>>  1053 |    fprintf(stderr, "cannot set log level %s,%d\n",
>>       |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>  1054 |     pattern, priority);
>>       |     ~~~~~~~~~~~~~~~~~~
>>
>
>Good catch.
>
>Just to understand, how did you catch it?
>Extra cflags? specific compiler?
>
>
>> Fixes: 7f0bb634a140 ("log: add ability to match log type with
>> globbing")
>
>Missing Cc: stable.
>
>
>>
>> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
>> ---
>>  lib/librte_eal/common/eal_common_options.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_eal/common/eal_common_options.c
>> b/lib/librte_eal/common/eal_common_options.c
>> index 75974dd5b..525e51e7d 100644
>> --- a/lib/librte_eal/common/eal_common_options.c
>> +++ b/lib/librte_eal/common/eal_common_options.c
>> @@ -1046,7 +1046,7 @@ eal_parse_log_level(const char *arg)
>>         if (regex) {
>>                 if (rte_log_set_level_regexp(regex, priority) < 0) {
>>                         fprintf(stderr, "cannot set log level %s,%d\n",
>> -                               pattern, priority);
>> +                               regex, priority);
>>                         goto fail;
>>                 }
>>                 if (rte_log_save_regexp(regex, priority) < 0)
>> --
>> 2.17.1
>>
>
>Acked-by: David Marchand <david.marchand@redhat.com>
>
>
>
>--
>David Marchand
  
David Marchand March 3, 2020, 9:55 a.m. UTC | #3
On Thu, Feb 27, 2020 at 12:03 PM Sunil Kumar Kori <skori@marvell.com> wrote:
> We have two build machine with two different version of GCC.
> So the issue is reported on one setup only not on the other one.
> It looks like because of different version of GCC, this warning is caught.

f30 gcc 9 with -Wformat-overflow (tried level 1 and 2) does not catch this.
  

Patch

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 75974dd5b..525e51e7d 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -1046,7 +1046,7 @@  eal_parse_log_level(const char *arg)
 	if (regex) {
 		if (rte_log_set_level_regexp(regex, priority) < 0) {
 			fprintf(stderr, "cannot set log level %s,%d\n",
-				pattern, priority);
+				regex, priority);
 			goto fail;
 		}
 		if (rte_log_save_regexp(regex, priority) < 0)