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:
extern __printf(3, 4)
void xfs_printk_level(const struct xfs_mount *mp, const char *fmt,
const char *fmt, ...);
There also needs to be an empty line between the prototype and the
following defines - the empty line provides demarcation between
prototype and the macro definitions that then use it.
> +#define xfs_emerg(mp, fmt, ...) \
> + xfs_printk_level(KERN_EMERG, mp, fmt, ##__VA_ARGS__)
> +#define xfs_alert(mp, fmt, ...) \
> + xfs_printk_level(KERN_ALERT, mp, fmt, ##__VA_ARGS__)
> +#define xfs_crit(mp, fmt, ...) \
> + xfs_printk_level(KERN_CRIT, mp, fmt, ##__VA_ARGS__)
> +#define xfs_err(mp, fmt, ...) \
> + xfs_printk_level(KERN_ERR, mp, fmt, ##__VA_ARGS__)
> +#define xfs_warn(mp, fmt, ...) \
> + xfs_printk_level(KERN_WARNING, mp, fmt, ##__VA_ARGS__)
> +#define xfs_notice(mp, fmt, ...) \
> + xfs_printk_level(KERN_NOTICE, mp, fmt, ##__VA_ARGS__)
> +#define xfs_info(mp, fmt, ...) \
> + xfs_printk_level(KERN_INFO, mp, fmt, ##__VA_ARGS__)
> #ifdef DEBUG
> -extern __printf(2, 3)
> -void xfs_debug(const struct xfs_mount *mp, const char *fmt, ...);
> +#define xfs_debug(mp, fmt, ...) \
> + xfs_printk_level(KERN_DEBUG, mp, fmt, ##__VA_ARGS__)
> #else
> -static inline __printf(2, 3)
> -void xfs_debug(const struct xfs_mount *mp, const char *fmt, ...)
> -{
> -}
> +#define xfs_debug(mp, fmt, ...) do {} while (0)
> #endif
>
> +extern __printf(3, 4)
> +void xfs_alert_tag(const struct xfs_mount *mp, int tag, const char *fmt, ...);
> +
> +
And one to many extra blank lines here :)
CHeers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
