RE: [ndclt PATCH] ndctl, monitor: Fix duplicate prefix in monitor.log

["Qi, Fuli" <qi.fuli@jp.fujitsu.com>]

> -----Original Message-----
> From: Masayoshi Mizuma [mailto:msys.mizuma@gmail.com]
> Sent: Thursday, August 2, 2018 3:15 AM
> To: Qi, Fuli/斉 福利 <qi.fuli@jp.fujitsu.com>; linux-nvdimm@lists.01.org
> Subject: Re: [ndclt PATCH] ndctl, monitor: Fix duplicate prefix in monitor.log
> 
> Hi QI,
> 
> On 07/31/2018 01:15 AM, QI Fuli wrote:
> > When a monitor runs with [--log] option, the prefix will be dually
> > added to monitor.log. Therefore, the monitor cannot log the smart
> > notification to syslog. This patch is used to prevent prefix from
> > being dually added to monitor.log.
> >
> > Fixes: fdf6b6844ccf ("ndctl, monitor: add a new command - monitor")
> > Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com>
> > ---
> >  ndctl/monitor.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/ndctl/monitor.c b/ndctl/monitor.c index c6419ad..4e5daf5
> > 100644
> > --- a/ndctl/monitor.c
> > +++ b/ndctl/monitor.c
> > @@ -614,7 +614,8 @@ int cmd_monitor(int argc, const char **argv, void *ctx)
> >  		goto out;
> >
> >  	if (monitor.log) {
> > -		fix_filename(prefix, (const char **)&monitor.log);
> > +		if (strncmp(monitor.log, "./", 2) != 0)
> > +			fix_filename(prefix, (const char **)&monitor.log);
> 
> prefix is not needed to 'syslog' and 'standard', so why don't you move the strncmp()
> before fix_filename(), like as:
> 
> @@ -614,13 +619,14 @@ int cmd_monitor(int argc, const char **argv, void *ctx)
>                 goto out;
> 
>         if (monitor.log) {
> -               fix_filename(prefix, (const char **)&monitor.log);
>                 if (strncmp(monitor.log, "./syslog", 8) == 0)
>                         ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog);
>                 else if (strncmp(monitor.log, "./standard", 10) == 0)
>                         ; /*default, already set */
> -               else
> +               else {
> +                       fix_filename(prefix, (const char
> + **)&monitor.log);
>                         ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_file);
> +               }
>         }
> 
>         if (monitor.daemon) {
> 
> Thanks,
> Masa

Hi Masa,

Thank you very much for your comments.

There are two ways to set monitor.log.
a) setting the argument of [--log] option
b) setting the value of [log] key in configuration file

When users set monitor.log by b, the prefix will not be added to monitor.log.
Therefore, we should do fix_filename() before strncmp().

Thanks,
QI

> 
> >  		if (strncmp(monitor.log, "./syslog", 8) == 0)
> >  			ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog);
> >  		else if (strncmp(monitor.log, "./standard", 10) == 0)
> >
> 

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm