[alsa-devel] [PATCH] Sound: drivers: virmidi: fixed code style issues
Fixed some minor code style issues and also removed some assignments inside of if conditionals. Signed-off-by: Kyle Chamberlin <KyleChamberlin@project20million.org> --- sound/drivers/virmidi.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/sound/drivers/virmidi.c b/sound/drivers/virmidi.c index b17872429..0af88d9 100644 --- a/sound/drivers/virmidi.c +++ b/sound/drivers/virmidi.c @@ -99,30 +99,33 @@ static int snd_virmidi_probe(struct platform_device *devptr) if (midi_devs[dev] > MAX_MIDI_DEVICES) { snd_printk(KERN_WARNING - "too much midi devices for virmidi %d: " - "force to use %d\n", dev, MAX_MIDI_DEVICES); + "too much midi devices for virmidi %d: force to use %d\n", + dev, MAX_MIDI_DEVICES); midi_devs[dev] = MAX_MIDI_DEVICES; } for (idx = 0; idx < midi_devs[dev]; idx++) { struct snd_rawmidi *rmidi; struct snd_virmidi_dev *rdev; - if ((err = snd_virmidi_new(card, idx, &rmidi)) < 0) + + err = snd_virmidi_new(card, idx, &rmidi); + if (err < 0) goto __nodev; rdev = rmidi->private_data; vmidi->midi[idx] = rmidi; strcpy(rmidi->name, "Virtual Raw MIDI"); rdev->seq_mode = SNDRV_VIRMIDI_SEQ_DISPATCH; } - + strcpy(card->driver, "VirMIDI"); strcpy(card->shortname, "VirMIDI"); sprintf(card->longname, "Virtual MIDI Card %i", dev + 1); - if ((err = snd_card_register(card)) == 0) { + err = snd_card_register(card); + if (err) { platform_set_drvdata(devptr, card); return 0; } - __nodev: +__nodev: snd_card_free(card); return err; } @@ -157,13 +160,15 @@ static int __init alsa_card_virmidi_init(void) { int i, cards, err; - if ((err = platform_driver_register(&snd_virmidi_driver)) < 0) + err = platform_driver_register(&snd_virmidi_driver); + if (err < 0) return err; cards = 0; for (i = 0; i < SNDRV_CARDS; i++) { struct platform_device *device; - if (! enable[i]) + + if (!enable[i]) continue; device = platform_device_register_simple(SND_VIRMIDI_DRIVER, i, NULL, 0); -- 2.1.3
At Fri, 28 Nov 2014 13:59:56 -0500, Kyle Chamberlin wrote:
Fixed some minor code style issues and also removed some assignments inside of if conditionals.
Signed-off-by: Kyle Chamberlin <KyleChamberlin@project20million.org>
Thanks, applied. But, it's better to keep your both from and sign-off addresses same at the next time. Takashi
--- sound/drivers/virmidi.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/sound/drivers/virmidi.c b/sound/drivers/virmidi.c index b17872429..0af88d9 100644 --- a/sound/drivers/virmidi.c +++ b/sound/drivers/virmidi.c @@ -99,30 +99,33 @@ static int snd_virmidi_probe(struct platform_device *devptr)
if (midi_devs[dev] > MAX_MIDI_DEVICES) { snd_printk(KERN_WARNING - "too much midi devices for virmidi %d: " - "force to use %d\n", dev, MAX_MIDI_DEVICES); + "too much midi devices for virmidi %d: force to use %d\n", + dev, MAX_MIDI_DEVICES); midi_devs[dev] = MAX_MIDI_DEVICES; } for (idx = 0; idx < midi_devs[dev]; idx++) { struct snd_rawmidi *rmidi; struct snd_virmidi_dev *rdev; - if ((err = snd_virmidi_new(card, idx, &rmidi)) < 0) + + err = snd_virmidi_new(card, idx, &rmidi); + if (err < 0) goto __nodev; rdev = rmidi->private_data; vmidi->midi[idx] = rmidi; strcpy(rmidi->name, "Virtual Raw MIDI"); rdev->seq_mode = SNDRV_VIRMIDI_SEQ_DISPATCH; } - + strcpy(card->driver, "VirMIDI"); strcpy(card->shortname, "VirMIDI"); sprintf(card->longname, "Virtual MIDI Card %i", dev + 1);
- if ((err = snd_card_register(card)) == 0) { + err = snd_card_register(card); + if (err) { platform_set_drvdata(devptr, card); return 0; } - __nodev: +__nodev: snd_card_free(card); return err; } @@ -157,13 +160,15 @@ static int __init alsa_card_virmidi_init(void) { int i, cards, err;
- if ((err = platform_driver_register(&snd_virmidi_driver)) < 0) + err = platform_driver_register(&snd_virmidi_driver); + if (err < 0) return err;
cards = 0; for (i = 0; i < SNDRV_CARDS; i++) { struct platform_device *device; - if (! enable[i]) + + if (!enable[i]) continue; device = platform_device_register_simple(SND_VIRMIDI_DRIVER, i, NULL, 0); -- 2.1.3
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@perches.com> --- 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__) #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);
On 2014年11月30日 05:58, 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@perches.com> --- include/sound/core.h | 28 +++++++++++++++------------- sound/core/misc.c | 6 +++--- 2 files changed, 18 insertions(+), 16 deletions(-)
I think it better to split this patch into two parts, one is for enumeration identifier and another is for variadic macro.
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__) #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);
There's no need to rename variable names. Regards Takashi Sakamoto o-takashi@sakamocchi.jp
On Sun, 2014-11-30 at 13:52 +0900, Takashi Sakamoto wrote:
On 2014年11月30日 05:58, Joe Perches wrote:
There's an enum defined for these magic numbers, might as well use it. [] I think it better to split this patch into two parts, one is for enumeration identifier and another is for variadic macro.
Go ahead then.
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@perches.com>
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);
On Sun, 2014-11-30 at 09:41 +0100, Takashi Iwai wrote:
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@perches.com>
Any specific reason to hang to an irrelevant thread?
Not in particular, I just noticed it there.
Also...
diff --git a/include/sound/core.h b/include/sound/core.h [] +#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.
Write it as you chose. It is pretty trivial.
At Sun, 30 Nov 2014 00:55:32 -0800, Joe Perches wrote:
On Sun, 2014-11-30 at 09:41 +0100, Takashi Iwai wrote:
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@perches.com>
Any specific reason to hang to an irrelevant thread?
Not in particular, I just noticed it there.
Also...
diff --git a/include/sound/core.h b/include/sound/core.h [] +#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.
Write it as you chose. It is pretty trivial.
... trivial for anyone ;) Takashi
On Sun, 2014-11-30 at 10:04 +0100, Takashi Iwai wrote:
At Sun, 30 Nov 2014 00:55:32 -0800, Joe Perches wrote:
On Sun, 2014-11-30 at 09:41 +0100, Takashi Iwai wrote:
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__ [] Write it as you chose. It is pretty trivial.
... trivial for anyone ;)
The other option is simply to remove the enum as the values are unused.
participants (4)
-
Joe Perches -
Kyle Chamberlin -
Takashi Iwai -
Takashi Sakamoto