On Tue, 15 Oct 2019 20:41:20 -0400
Joel Fernandes <j...@joelfernandes.org> wrote:
> > +static const struct file_operations tracing_max_lat_fops;
> > +
> > +#if (defined(CONFIG_TRACER_MAX_TRACE) || defined(CONFIG_HWLAT_TRACER)) && \
> > + defined(CONFIG_FSNOTIFY)
>
> Something bothers me. If you dropped support for HWLAT_TRACER as you
> mentioned in the cover letter, then why does this #if look for the CONFIG
> option?
>
> (and similar comment on the rest of the patch..)
Also, if you have a complex if statement like this, it is better to
create a new define to set it in a header file:
#if (defined(CONFIG_TRACER_MAX_TRACE) || defined(CONFIG_HWLAT_TRACER)) && \
defined(CONFIG_FSNOTIFY)
# define HAVE_TRACER_FSNOTIFY
#endif
And then just use that:
#ifdef HAVE_TRACER_FSNOTIFY
...
#endif
It keeps things a bit more manageable.
-- Steve
