Re: [PATCH] sound: use enum names instead of magic numbers

From: Takashi Iwai
Date: Sun Nov 30 2014 - 03:41:59 EST


At Sat, 29 Nov 2014 12:58:12 -0800,
Joe Perches wrote:
>
> There's an enum defined for these magic numbers,
> might as well use it.
>
> Miscellanea:
> o Use ##__VA_ARGS__
>
> Signed-off-by: Joe Perches <joe@xxxxxxxxxxx>

Any specific reason to hang to an irrelevant thread?

Also...

> ---
> include/sound/core.h | 28 +++++++++++++++-------------
> sound/core/misc.c | 6 +++---
> 2 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/include/sound/core.h b/include/sound/core.h
> index 1df3f2f..40418e7 100644
> --- a/include/sound/core.h
> +++ b/include/sound/core.h
> @@ -325,7 +325,7 @@ void release_and_free_resource(struct resource *res);
> /* --- */
>
> /* sound printk debug levels */
> -enum {
> +enum snd_level {
> SND_PR_ALWAYS,
> SND_PR_DEBUG,
> SND_PR_VERBOSE,
> @@ -333,11 +333,11 @@ enum {
>
> #if defined(CONFIG_SND_DEBUG) || defined(CONFIG_SND_VERBOSE_PRINTK)
> __printf(4, 5)
> -void __snd_printk(unsigned int level, const char *file, int line,
> +void __snd_printk(enum snd_level snd_level, const char *file, int line,
> const char *format, ...);
> #else
> -#define __snd_printk(level, file, line, format, args...) \
> - printk(format, ##args)
> +#define __snd_printk(snd_level, file, line, format, ...) \
> + printk(format, ##__VA_ARGS__)

It's better to keep the argument name "level". Using both the same
name for a variable and its enum type is rather confusing, and just
gives unnecessary LOCs.


thanks,

Takashi

> #endif
>
> /**
> @@ -347,8 +347,8 @@ void __snd_printk(unsigned int level, const char *file, int line,
> * Works like printk() but prints the file and the line of the caller
> * when configured with CONFIG_SND_VERBOSE_PRINTK.
> */
> -#define snd_printk(fmt, args...) \
> - __snd_printk(0, __FILE__, __LINE__, fmt, ##args)
> +#define snd_printk(fmt, ...) \
> + __snd_printk(SND_PR_ALWAYS, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
>
> #ifdef CONFIG_SND_DEBUG
> /**
> @@ -358,10 +358,10 @@ void __snd_printk(unsigned int level, const char *file, int line,
> * Works like snd_printk() for debugging purposes.
> * Ignored when CONFIG_SND_DEBUG is not set.
> */
> -#define snd_printd(fmt, args...) \
> - __snd_printk(1, __FILE__, __LINE__, fmt, ##args)
> -#define _snd_printd(level, fmt, args...) \
> - __snd_printk(level, __FILE__, __LINE__, fmt, ##args)
> +#define snd_printd(fmt, ...) \
> + __snd_printk(SND_PR_DEBUG, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
> +#define _snd_printd(level, fmt, ...) \
> + __snd_printk(level, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
>
> /**
> * snd_BUG - give a BUG warning message and stack trace
> @@ -390,7 +390,8 @@ void __snd_printk(unsigned int level, const char *file, int line,
> __printf(1, 2)
> static inline void snd_printd(const char *format, ...) {}
> __printf(2, 3)
> -static inline void _snd_printd(int level, const char *format, ...) {}
> +static inline void _snd_printd(enum snd_level snd_level,
> + const char *format, ...) {}
>
> #define snd_BUG() do { } while (0)
>
> @@ -411,8 +412,9 @@ static inline bool snd_printd_ratelimit(void) { return false; }
> * Works like snd_printk() for debugging purposes.
> * Ignored when CONFIG_SND_DEBUG_VERBOSE is not set.
> */
> -#define snd_printdd(format, args...) \
> - __snd_printk(2, __FILE__, __LINE__, format, ##args)
> +#define snd_printdd(format, ...) \
> + __snd_printk(SND_PR_VERBOSE, __FILE__, __LINE__, \
> + format, ##__VA_ARGS__)
> #else
> __printf(1, 2)
> static inline void snd_printdd(const char *format, ...) {}
> diff --git a/sound/core/misc.c b/sound/core/misc.c
> index f2e8226..03b3f56 100644
> --- a/sound/core/misc.c
> +++ b/sound/core/misc.c
> @@ -63,7 +63,7 @@ static const char *sanity_file_name(const char *path)
> #endif
>
> #if defined(CONFIG_SND_DEBUG) || defined(CONFIG_SND_VERBOSE_PRINTK)
> -void __snd_printk(unsigned int level, const char *path, int line,
> +void __snd_printk(enum snd_level snd_level, const char *path, int line,
> const char *format, ...)
> {
> va_list args;
> @@ -74,7 +74,7 @@ void __snd_printk(unsigned int level, const char *path, int line,
> #endif
>
> #ifdef CONFIG_SND_DEBUG
> - if (debug < level)
> + if (debug < snd_level)
> return;
> #endif
>
> @@ -88,7 +88,7 @@ void __snd_printk(unsigned int level, const char *path, int line,
> const char *end_of_header = printk_skip_level(format);
> memcpy(verbose_fmt, format, end_of_header - format);
> vaf.fmt = end_of_header;
> - } else if (level)
> + } else if (snd_level)
> memcpy(verbose_fmt, KERN_DEBUG, sizeof(KERN_DEBUG) - 1);
> printk(verbose_fmt, sanity_file_name(path), line, &vaf);
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/