| Message ID | 20230513142038.753351-3-lizhijian@fujitsu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl/monitor and ndctl/monitor fixes | expand |
On 5/13/23 7:20 AM, Li Zhijian wrote: > 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> > --- > cxl/monitor.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/cxl/monitor.c b/cxl/monitor.c > index 4043928db3ef..842e54b186ab 100644 > --- a/cxl/monitor.c > +++ b/cxl/monitor.c > @@ -181,7 +181,8 @@ int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx) > if (monitor.log) { > if (strncmp(monitor.log, "./", 2) != 0) > fix_filename(prefix, (const char **)&monitor.log); > - if (strncmp(monitor.log, "./standard", 10) == 0 && !monitor.daemon) { > + > + if (strcmp(monitor.log, "./standard") == 0 && !monitor.daemon) { The code change doesn't match the commit log. Here it just changed from strncmp() to strcmp(). Please explain what's going on here. > monitor.ctx.log_fn = log_standard; > } else { > const char *log = monitor.log;
Dave On 20/05/2023 01:31, Dave Jiang wrote: > > > On 5/13/23 7:20 AM, Li Zhijian wrote: >> 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> >> --- >> cxl/monitor.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/cxl/monitor.c b/cxl/monitor.c >> index 4043928db3ef..842e54b186ab 100644 >> --- a/cxl/monitor.c >> +++ b/cxl/monitor.c >> @@ -181,7 +181,8 @@ int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx) >> if (monitor.log) { >> if (strncmp(monitor.log, "./", 2) != 0) >> fix_filename(prefix, (const char **)&monitor.log); >> - if (strncmp(monitor.log, "./standard", 10) == 0 && !monitor.daemon) { >> + >> + if (strcmp(monitor.log, "./standard") == 0 && !monitor.daemon) { > > The code change doesn't match the commit log. Here it just changed from strncmp() to strcmp(). Please explain what's going on here. > Okay, i will update more in the commit log. something like: cxl/monitor: use strcmp to compare the reserved word According to its document, when '-l standard' is specified, log would be output to the stdout. But actually, since it's using strncmp(a, b, 10) to compare the former 10 characters, it will also wrongly treat a filename starting with a substring 'standard' to 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> --- V2: commit log updated # Dave Thanks Zhijian >> monitor.ctx.log_fn = log_standard; >> } else { >> const char *log = monitor.log; >
On 5/21/23 11:08 PM, Zhijian Li (Fujitsu) wrote: > Dave > > > On 20/05/2023 01:31, Dave Jiang wrote: >> >> >> On 5/13/23 7:20 AM, Li Zhijian wrote: >>> 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> >>> --- >>> cxl/monitor.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/cxl/monitor.c b/cxl/monitor.c >>> index 4043928db3ef..842e54b186ab 100644 >>> --- a/cxl/monitor.c >>> +++ b/cxl/monitor.c >>> @@ -181,7 +181,8 @@ int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx) >>> if (monitor.log) { >>> if (strncmp(monitor.log, "./", 2) != 0) >>> fix_filename(prefix, (const char **)&monitor.log); >>> - if (strncmp(monitor.log, "./standard", 10) == 0 && !monitor.daemon) { >>> + >>> + if (strcmp(monitor.log, "./standard") == 0 && !monitor.daemon) { >> >> The code change doesn't match the commit log. Here it just changed from strncmp() to strcmp(). Please explain what's going on here. >> > > > Okay, i will update more in the commit log. something like: > > cxl/monitor: use strcmp to compare the reserved word > > According to its document, when '-l standard' is specified, log would be s/its document/the tool's documentation/ > output to the stdout. But actually, since it's using strncmp(a, b, 10) s/But actually, since/But since/ > to compare the former 10 characters, it will also wrongly treat a filename s/treat/detect/ > starting with a substring 'standard' to stdout. s/to/as/ > > 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> > --- > V2: commit log updated # Dave This makes it significantly clearer. Thank you. > > > Thanks > Zhijian > > > >>> monitor.ctx.log_fn = log_standard; >>> } else { >>> const char *log = monitor.log;
On 22/05/2023 23:15, Dave Jiang wrote: > > > On 5/21/23 11:08 PM, Zhijian Li (Fujitsu) wrote: >> Dave >> >> >> On 20/05/2023 01:31, Dave Jiang wrote: >>> >>> >>> On 5/13/23 7:20 AM, Li Zhijian wrote: >>>> 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> >>>> --- >>>> cxl/monitor.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/cxl/monitor.c b/cxl/monitor.c >>>> index 4043928db3ef..842e54b186ab 100644 >>>> --- a/cxl/monitor.c >>>> +++ b/cxl/monitor.c >>>> @@ -181,7 +181,8 @@ int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx) >>>> if (monitor.log) { >>>> if (strncmp(monitor.log, "./", 2) != 0) >>>> fix_filename(prefix, (const char **)&monitor.log); >>>> - if (strncmp(monitor.log, "./standard", 10) == 0 && !monitor.daemon) { >>>> + >>>> + if (strcmp(monitor.log, "./standard") == 0 && !monitor.daemon) { >>> >>> The code change doesn't match the commit log. Here it just changed from strncmp() to strcmp(). Please explain what's going on here. >>> >> >> >> Okay, i will update more in the commit log. something like: >> >> cxl/monitor: use strcmp to compare the reserved word >> According to its document, when '-l standard' is specified, log would be > > s/its document/the tool's documentation/ > >> output to the stdout. But actually, since it's using strncmp(a, b, 10) > > s/But actually, since/But since/ > >> to compare the former 10 characters, it will also wrongly treat a filename > > s/treat/detect/ > >> starting with a substring 'standard' to stdout. > > s/to/as/ > >> 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> >> --- >> V2: commit log updated # Dave > > This makes it significantly clearer. Thank you. Sorry for the delay. your comments are good to me. thank you very much. I have addressed them in my local V3 branch which will be sent if there is no more comments to my V2 set in next week. Thanks Zhijian > > >> >> >> Thanks >> Zhijian >> >> >> >>>> monitor.ctx.log_fn = log_standard; >>>> } else { >>>> const char *log = monitor.log;
diff --git a/cxl/monitor.c b/cxl/monitor.c index 4043928db3ef..842e54b186ab 100644 --- a/cxl/monitor.c +++ b/cxl/monitor.c @@ -181,7 +181,8 @@ int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx) if (monitor.log) { if (strncmp(monitor.log, "./", 2) != 0) fix_filename(prefix, (const char **)&monitor.log); - if (strncmp(monitor.log, "./standard", 10) == 0 && !monitor.daemon) { + + if (strcmp(monitor.log, "./standard") == 0 && !monitor.daemon) { monitor.ctx.log_fn = log_standard; } else { const char *log = monitor.log;
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> --- cxl/monitor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)