Skip to contentSkip to conversation

Sound: drivers: virmidi: fixed code style issues

PATCH Applied Kyle Chamberlin 4 participants 9 messages Nov 28, 2014 – Nov 30, 2014 Sound

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Sound: drivers: virmidi: fixed code style issues

Fixed some minor code style issues and also removed some assignments inside of if conditionals.

---
 sound/drivers/virmidi.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
sound/drivers/virmidi.c +13 -8
index b17872429..0af88d9 100644
@@ -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);
Signature · 1 line
2.1.3
Applied
#1 lore raw

At Fri, 28 Nov 2014 13:59:56 -0500,

Quoted 5 lines · Kyle Chamberlin

Fixed some minor code style issues and also removed some assignments inside of if conditionals.

Thanks, applied.

But, it's better to keep your both from and sign-off addresses same at the next time.

Takashi

Quoted 69 lines
---
 sound/drivers/virmidi.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
sound/drivers/virmidi.c +13 -8
index b17872429..0af88d9 100644
@@ -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);
Signature · 1 line
2.1.3
#2 lore raw

PATCH 1/1 sound: use enum names instead of magic numbers

There's an enum defined for these magic numbers,
might as well use it.

Miscellanea:
o Use ##__VA_ARGS__

---
 include/sound/core.h | 28 +++++++++++++++-------------
 sound/core/misc.c    |  6 +++---
 2 files changed, 18 insertions(+), 16 deletions(-)
include/sound/core.h +15 -13
index 1df3f2f..40418e7 100644
@@ -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, ...) {}
sound/core/misc.c +3 -3
index f2e8226..03b3f56 100644
@@ -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);
 
#3 lore raw

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

Quoted 11 lines · Joe Perches

There's an enum defined for these magic numbers,
might as well use it.

Miscellanea:
o Use ##__VA_ARGS__

---
 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.

Quoted 98 lines
include/sound/core.h +15 -13
index 1df3f2f..40418e7 100644
@@ -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, ...) {}
sound/core/misc.c +2 -2
index f2e8226..03b3f56 100644
@@ -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
 

sound/core/misc.c void __snd_printk(unsigned int level, const char *path, int line, · line 93

Quoted 7 lines
sound/core/misc.c +1 -1
@@ -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
[email protected]

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

#4 lore raw

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

Quoted 3 lines · Takashi Sakamoto
Quoted 2 lines · Joe Perches

There's an enum defined for these magic numbers,
might as well use it.

[]

Quoted 2 lines

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.

#5 lore raw

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

In reply to Joe Perches

At Sat, 29 Nov 2014 12:58:12 -0800,

Quoted 8 lines · Joe Perches

There's an enum defined for these magic numbers,
might as well use it.

Miscellanea:
o Use ##__VA_ARGS__

Any specific reason to hang to an irrelevant thread?

Also...

Quoted 20 lines
---
 include/sound/core.h | 28 +++++++++++++++-------------
 sound/core/misc.c    |  6 +++---
 2 files changed, 18 insertions(+), 16 deletions(-)
include/sound/core.h +1 -1
index 1df3f2f..40418e7 100644
@@ -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 {
 

include/sound/core.h enum { · line 340

Quoted 10 lines
include/sound/core.h +3 -3
 #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

Quoted 84 lines
include/sound/core.h +11 -9
 #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, ...) {}
sound/core/misc.c +3 -3
index f2e8226..03b3f56 100644
@@ -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);
 
#6 lore raw

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

Quoted 6 lines · Takashi Iwai

At Sat, 29 Nov 2014 12:58:12 -0800,

Quoted 4 lines · Joe Perches

There's an enum defined for these magic numbers,
might as well use it.

Quoted 6 lines
Quoted 4 lines

Miscellanea:
o Use ##__VA_ARGS__

Any specific reason to hang to an irrelevant thread?

Not in particular, I just noticed it there.

Quoted 3 lines

Also...

include/sound/core.h

Quoted 1 line
include/sound/core.h +0 -0

[]

Quoted 6 lines

include/sound/core.h enum { · line 340

Quoted 2 lines
include/sound/core.h +2 -0
+#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.

#7 lore raw

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

At Sun, 30 Nov 2014 00:55:32 -0800,

Quoted 21 lines · Joe Perches
Quoted 6 lines · Takashi Iwai

At Sat, 29 Nov 2014 12:58:12 -0800,

Quoted 4 lines · Joe Perches

There's an enum defined for these magic numbers,
might as well use it.

Quoted 6 lines
Quoted 4 lines

Miscellanea:
o Use ##__VA_ARGS__

Any specific reason to hang to an irrelevant thread?

Not in particular, I just noticed it there.

Quoted 3 lines

Also...

include/sound/core.h

Quoted 1 line
include/sound/core.h +0 -0

[]

Quoted 8 lines
Quoted 6 lines

include/sound/core.h enum { · line 340

Quoted 2 lines
include/sound/core.h +2 -0
+#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

#8 lore raw

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

Quoted 8 lines · Takashi Iwai
Quoted 7 lines · Joe Perches
Quoted 6 lines · Takashi Iwai
Quoted 5 lines · Joe Perches

There's an enum defined for these magic numbers,
might as well use it.

Miscellanea:
o Use ##__VA_ARGS__

[]

Quoted 3 lines
Quoted 1 line

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.