* [PATCH] Sound: drivers: virmidi: fixed code style issues @ 2014-11-28 18:59 Kyle Chamberlin 2014-11-29 20:36 ` Takashi Iwai 0 siblings, 1 reply; 9+ messages in thread From: Kyle Chamberlin @ 2014-11-28 18:59 UTC (permalink / raw) To: perex; +Cc: tiwai, alsa-devel, linux-kernel, Kyle Chamberlin 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Sound: drivers: virmidi: fixed code style issues 2014-11-28 18:59 [PATCH] Sound: drivers: virmidi: fixed code style issues Kyle Chamberlin @ 2014-11-29 20:36 ` Takashi Iwai 2014-11-29 20:58 ` [PATCH] sound: use enum names instead of magic numbers Joe Perches 0 siblings, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2014-11-29 20:36 UTC (permalink / raw) To: Kyle Chamberlin; +Cc: perex, alsa-devel, linux-kernel, Kyle Chamberlin 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] sound: use enum names instead of magic numbers 2014-11-29 20:36 ` Takashi Iwai @ 2014-11-29 20:58 ` Joe Perches 2014-11-30 4:52 ` Takashi Sakamoto 2014-11-30 8:41 ` Takashi Iwai 0 siblings, 2 replies; 9+ messages in thread From: Joe Perches @ 2014-11-29 20:58 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel, linux-kernel 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); ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] sound: use enum names instead of magic numbers 2014-11-29 20:58 ` [PATCH] sound: use enum names instead of magic numbers Joe Perches @ 2014-11-30 4:52 ` Takashi Sakamoto 2014-11-30 4:57 ` Joe Perches 2014-11-30 8:41 ` Takashi Iwai 1 sibling, 1 reply; 9+ messages in thread From: Takashi Sakamoto @ 2014-11-30 4:52 UTC (permalink / raw) To: Joe Perches, Takashi Iwai; +Cc: alsa-devel, linux-kernel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] sound: use enum names instead of magic numbers 2014-11-30 4:52 ` Takashi Sakamoto @ 2014-11-30 4:57 ` Joe Perches 0 siblings, 0 replies; 9+ messages in thread From: Joe Perches @ 2014-11-30 4:57 UTC (permalink / raw) To: Takashi Sakamoto; +Cc: Takashi Iwai, alsa-devel, linux-kernel 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] sound: use enum names instead of magic numbers 2014-11-29 20:58 ` [PATCH] sound: use enum names instead of magic numbers Joe Perches 2014-11-30 4:52 ` Takashi Sakamoto @ 2014-11-30 8:41 ` Takashi Iwai 2014-11-30 8:55 ` Joe Perches 1 sibling, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2014-11-30 8:41 UTC (permalink / raw) To: Joe Perches; +Cc: alsa-devel, linux-kernel 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); > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] sound: use enum names instead of magic numbers 2014-11-30 8:41 ` Takashi Iwai @ 2014-11-30 8:55 ` Joe Perches 2014-11-30 9:04 ` Takashi Iwai 0 siblings, 1 reply; 9+ messages in thread From: Joe Perches @ 2014-11-30 8:55 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel, linux-kernel 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] sound: use enum names instead of magic numbers 2014-11-30 8:55 ` Joe Perches @ 2014-11-30 9:04 ` Takashi Iwai 2014-11-30 19:26 ` Joe Perches 0 siblings, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2014-11-30 9:04 UTC (permalink / raw) To: Joe Perches; +Cc: alsa-devel, linux-kernel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] sound: use enum names instead of magic numbers 2014-11-30 9:04 ` Takashi Iwai @ 2014-11-30 19:26 ` Joe Perches 0 siblings, 0 replies; 9+ messages in thread From: Joe Perches @ 2014-11-30 19:26 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel, linux-kernel 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-11-30 19:26 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-11-28 18:59 [PATCH] Sound: drivers: virmidi: fixed code style issues Kyle Chamberlin 2014-11-29 20:36 ` Takashi Iwai 2014-11-29 20:58 ` [PATCH] sound: use enum names instead of magic numbers Joe Perches 2014-11-30 4:52 ` Takashi Sakamoto 2014-11-30 4:57 ` Joe Perches 2014-11-30 8:41 ` Takashi Iwai 2014-11-30 8:55 ` Joe Perches 2014-11-30 9:04 ` Takashi Iwai 2014-11-30 19:26 ` Joe Perches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox