On Wed 2022-03-30 10:54:41, Dave Chinner wrote:
> On Fri, Mar 25, 2022 at 10:19:45AM -0700, Jonathan Lassoff wrote:
> > Rather than have a constructor to define many nearly-identical
> > functions, use preprocessor macros to pass down a kernel logging level
> > to a common function.
> >
> > Signed-off-by: Jonathan Lassoff <jof@xxxxxxxxxx>
>
> Mostly looks good, mainly just whitespace/formatting consistency
> issues now.
>
> ....
> > -define_xfs_printk_level(xfs_emerg, KERN_EMERG);
> > -define_xfs_printk_level(xfs_alert, KERN_ALERT);
> > -define_xfs_printk_level(xfs_crit, KERN_CRIT);
> > -define_xfs_printk_level(xfs_err, KERN_ERR);
> > -define_xfs_printk_level(xfs_warn, KERN_WARNING);
> > -define_xfs_printk_level(xfs_notice, KERN_NOTICE);
> > -define_xfs_printk_level(xfs_info, KERN_INFO);
> > -#ifdef DEBUG
> > -define_xfs_printk_level(xfs_debug, KERN_DEBUG);
> > -#endif
> > +void xfs_printk_level(
> > + const char *kern_level,
> > + const struct xfs_mount *mp,
> > + const char *fmt, ...)
>
> Use the same format as __xfs_printk() and xfs_alert_tag():
>
> void
> xfs_printk_level(
> const char *kern_level,
> const struct xfs_mount *mp,
> const char *fmt, ...)
>
> > +{
> > + struct va_format vaf;
> > + va_list args;
> > + int level;
> > +
> > + va_start(args, fmt);
> > + vaf.fmt = fmt;
> > + vaf.va = &args;
> > +
> > + __xfs_printk(kern_level, mp, &vaf);
> > +
> > + va_end(args);
> > +
> > + if (!kstrtoint(kern_level, 0, &level) &&
> > + level <= LOGLEVEL_ERR &&
> > + xfs_error_level >= XFS_ERRLEVEL_HIGH)
> > + xfs_stack_trace();
> > +}
> >
> > void
> > xfs_alert_tag(
> > diff --git a/fs/xfs/xfs_message.h b/fs/xfs/xfs_message.h
> > index bb9860ec9a93..2f609800e806 100644
> > --- a/fs/xfs/xfs_message.h
> > +++ b/fs/xfs/xfs_message.h
> > @@ -6,33 +6,36 @@
> >
> > struct xfs_mount;
> >
> > -extern __printf(2, 3)
> > -void xfs_emerg(const struct xfs_mount *mp, const char *fmt, ...);
> > -extern __printf(2, 3)
> > -void xfs_alert(const struct xfs_mount *mp, const char *fmt, ...);
> > extern __printf(3, 4)
^^^^^^^
> > -void xfs_alert_tag(const struct xfs_mount *mp, int tag, const char *fmt, ...);
> > -extern __printf(2, 3)
> > -void xfs_crit(const struct xfs_mount *mp, const char *fmt, ...);
> > -extern __printf(2, 3)
> > -void xfs_err(const struct xfs_mount *mp, const char *fmt, ...);
> > -extern __printf(2, 3)
> > -void xfs_warn(const struct xfs_mount *mp, const char *fmt, ...);
> > -extern __printf(2, 3)
> > -void xfs_notice(const struct xfs_mount *mp, const char *fmt, ...);
> > -extern __printf(2, 3)
> > -void xfs_info(const struct xfs_mount *mp, const char *fmt, ...);
> > -
> > +void xfs_printk_level(
> > + const char *kern_level,
> > + const struct xfs_mount *mp,
> > + const char *fmt, ...);
>
> This still needs the __printf() attribute because we still want the
> compiler to check the printf format string for issues. Also the
> format for function prototypes should follow the ones that got
> removed:
It is actually there. But it is hidden in many removed lines.
BTW: I missed it when reading the patch as well. I was surprised
when I saw it after applying the patch ;-)
Best Regards,
Petr
