Re: [ndctl PATCH v4 3/4] ndctl: use strcmp for reserved word in monitor commands

From: Alison Schofield <alison.schofield@intel.com>
To: Li Zhijian <lizhijian@fujitsu.com>
Cc: nvdimm@lists.linux.dev, linux-cxl@vger.kernel.org,
	Dave Jiang <dave.jiang@intel.com>
Subject: Re: [ndctl PATCH v4 3/4] ndctl: use strcmp for reserved word in monitor commands
Date: Tue, 11 Jul 2023 10:58:11 -0700	[thread overview]
Message-ID: <ZK2YM7nlCrEmz1+9@aschofie-mobl2> (raw)
In-Reply-To: <20230711115344.562823-4-lizhijian@fujitsu.com>

On Tue, Jul 11, 2023 at 07:53:43PM +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.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

> 
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
> V4: combine ndctl/monitor to one patch
> V3: Improve commit log # Dave
> V2: commit log updated # Dave
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
>  cxl/monitor.c   | 2 +-
>  ndctl/monitor.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/cxl/monitor.c b/cxl/monitor.c
> index e83455b63d35..a85452a4dc82 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+");
> diff --git a/ndctl/monitor.c b/ndctl/monitor.c
> index 89903def63d4..bd8a74863476 100644
> --- a/ndctl/monitor.c
> +++ b/ndctl/monitor.c
> @@ -610,9 +610,9 @@ int cmd_monitor(int argc, const char **argv, struct ndctl_ctx *ctx)
>  	if (monitor.log) {
>  		if (strncmp(monitor.log, "./", 2) != 0)
>  			fix_filename(prefix, (const char **)&monitor.log);
> -		if (strncmp(monitor.log, "./syslog", 8) == 0)
> +		if (strcmp(monitor.log, "./syslog") == 0)
>  			monitor.ctx.log_fn = log_syslog;
> -		else if (strncmp(monitor.log, "./standard", 10) == 0)
> +		else if (strcmp(monitor.log, "./standard") == 0)
>  			monitor.ctx.log_fn = log_standard;
>  		else {
>  			monitor.ctx.log_file = fopen(monitor.log, "a+");
> -- 
> 2.29.2
>