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

["Verma, Vishal L" <vishal.l.verma@intel.com>]

On Thu, 2018-08-02 at 09:22 -0400, Masayoshi Mizuma wrote:
> Hi QI,
> 
> On 08/02/2018 05:42 AM, Qi, Fuli wrote:
> ...
> > > > 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().
> 
> Oh, my proposal patch does not cover in case of user set 'syslog' or
> 'standard'in config_file.
> I think your patch fixes the bug correctly, thanks.
> 
> Please feel free to add:
> 
> Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

Thanks Qi, Masa - I've applied this.

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