| Message ID | 20230531021936.7366-4-lizhijian@fujitsu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl/monitor and ndctl/monitor fixes | expand |
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+");
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+");
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 --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+");
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(-)