diff mbox series

[ndctl,v3,3/6] cxl/monitor: use strcmp to compare the reserved word

Message ID 20230531021936.7366-4-lizhijian@fujitsu.com
State Superseded
Headers show
Series cxl/monitor and ndctl/monitor fixes | expand

Commit Message

Zhijian Li (Fujitsu) May 31, 2023, 2:19 a.m. UTC
According to the tool's documentation, when '-l standard' is specified,
log would be output to the stdout. But since it's using strncmp(a, b, 10)
to compare the former 10 characters, it will also wrongly detect a filename
starting with a substring 'standard' as stdout.

For example:
$ cxl monitor -l standard.log

User is most likely want to save log to ./standard.log instead of stdout.

Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---
V3: Improve commit log # Dave
V2: commit log updated # Dave
---
 cxl/monitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Dave Jiang July 5, 2023, 6:20 p.m. UTC | #1
On 5/30/23 19:19, Li Zhijian wrote:
> According to the tool's documentation, when '-l standard' is specified,
> log would be output to the stdout. But since it's using strncmp(a, b, 10)
> to compare the former 10 characters, it will also wrongly detect a filename
> starting with a substring 'standard' as stdout.
> 
> For example:
> $ cxl monitor -l standard.log
> 
> User is most likely want to save log to ./standard.log instead of stdout.
> 
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> V3: Improve commit log # Dave
> V2: commit log updated # Dave
> ---
>   cxl/monitor.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/cxl/monitor.c b/cxl/monitor.c
> index f0e3c4c3f45c..179646562187 100644
> --- a/cxl/monitor.c
> +++ b/cxl/monitor.c
> @@ -188,7 +188,7 @@ int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx)
>   	else
>   		monitor.ctx.log_priority = LOG_INFO;
>   
> -	if (strncmp(log, "./standard", 10) == 0)
> +	if (strcmp(log, "./standard") == 0)
>   		monitor.ctx.log_fn = log_standard;
>   	else {
>   		monitor.ctx.log_file = fopen(log, "a+");
Verma, Vishal L July 5, 2023, 9:03 p.m. UTC | #2
On Wed, 2023-05-31 at 10:19 +0800, Li Zhijian wrote:
> According to the tool's documentation, when '-l standard' is
> specified,
> log would be output to the stdout. But since it's using strncmp(a, b,
> 10)
> to compare the former 10 characters, it will also wrongly detect a
> filename
> starting with a substring 'standard' as stdout.
> 
> For example:
> $ cxl monitor -l standard.log
> 
> User is most likely want to save log to ./standard.log instead of
> stdout.
> 
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
> V3: Improve commit log # Dave
> V2: commit log updated # Dave
> ---
>  cxl/monitor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/cxl/monitor.c b/cxl/monitor.c
> index f0e3c4c3f45c..179646562187 100644
> --- a/cxl/monitor.c
> +++ b/cxl/monitor.c
> @@ -188,7 +188,7 @@ int cmd_monitor(int argc, const char **argv,
> struct cxl_ctx *ctx)
>         else
>                 monitor.ctx.log_priority = LOG_INFO;
>  
> -       if (strncmp(log, "./standard", 10) == 0)
> +       if (strcmp(log, "./standard") == 0)

As noted in patch 1, this should just be using 'standard', not
'./standard'.

With these patches the behavior becomes:

* if -l standard is used, it creates a file called standard in the cwd
* if -l ./standard is used, it uses stdout

It should behave the opposite way for both of those cases.

>                 monitor.ctx.log_fn = log_standard;
>         else {
>                 monitor.ctx.log_file = fopen(log, "a+");
Zhijian Li (Fujitsu) July 10, 2023, 12:53 p.m. UTC | #3
on 7/6/2023 5:03 AM, Verma, Vishal L wrote:
> On Wed, 2023-05-31 at 10:19 +0800, Li Zhijian wrote:
>> According to the tool's documentation, when '-l standard' is
>> specified,
>> log would be output to the stdout. But since it's using strncmp(a, b,
>> 10)
>> to compare the former 10 characters, it will also wrongly detect a
>> filename
>> starting with a substring 'standard' as stdout.
>>
>> For example:
>> $ cxl monitor -l standard.log
>>
>> User is most likely want to save log to ./standard.log instead of
>> stdout.
>>
>> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
>> ---
>> V3: Improve commit log # Dave
>> V2: commit log updated # Dave
>> ---
>>   cxl/monitor.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/cxl/monitor.c b/cxl/monitor.c
>> index f0e3c4c3f45c..179646562187 100644
>> --- a/cxl/monitor.c
>> +++ b/cxl/monitor.c
>> @@ -188,7 +188,7 @@ int cmd_monitor(int argc, const char **argv,
>> struct cxl_ctx *ctx)
>>          else
>>                  monitor.ctx.log_priority = LOG_INFO;
>>   
>> -       if (strncmp(log, "./standard", 10) == 0)
>> +       if (strcmp(log, "./standard") == 0)
> As noted in patch 1, this should just be using 'standard', not
> './standard'.
>
> With these patches the behavior becomes:
>
> * if -l standard is used, it creates a file called standard in the cwd
> * if -l ./standard is used, it uses stdout

Not really, './standard' will become '././standard'  by 
parse_options_prefix()

Thanks
Zhijian

>
> It should behave the opposite way for both of those cases.
>
>>                  monitor.ctx.log_fn = log_standard;
>>          else {
>>                  monitor.ctx.log_file = fopen(log, "a+");
diff mbox series

Patch

diff --git a/cxl/monitor.c b/cxl/monitor.c
index f0e3c4c3f45c..179646562187 100644
--- a/cxl/monitor.c
+++ b/cxl/monitor.c
@@ -188,7 +188,7 @@  int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx)
 	else
 		monitor.ctx.log_priority = LOG_INFO;
 
-	if (strncmp(log, "./standard", 10) == 0)
+	if (strcmp(log, "./standard") == 0)
 		monitor.ctx.log_fn = log_standard;
 	else {
 		monitor.ctx.log_file = fopen(log, "a+");