Skip to contentSkip to conversation

ALSA: pcm_native: remove unused label

PATCH In review Marcin Slusarz 2 participants 4 messages Aug 3, 2008 Sound

lore
Share & tools
lore

Marcin Slusarz

author
#0 lore raw

PATCH 1/1 ALSA: pcm_native: remove unused label

gcc warns about it:
sound/core/pcm_native.c: In function 'snd_pcm_fasync':
sound/core/pcm_native.c:3262: warning: label 'out' defined but not used

Signed-off-by Marcin Slusarz <[email protected]>
Cc Takashi Iwai <[email protected]>
Cc Jaroslav Kysela <[email protected]>
---
 sound/core/pcm_native.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
sound/core/pcm_native.c +0 -1
index c49b9d9..333cff6 100644
@@ -3259,7 +3259,6 @@ static int snd_pcm_fasync(int fd, struct file * file, int on)
 	runtime = substream->runtime;
 
 	err = fasync_helper(fd, file, on, &runtime->fasync);
-out:
 	unlock_kernel();
 	if (err < 0)
 		return err;
Signature · 1 line
1.5.4.5
#1 lore raw
Quoted 16 lines · Marcin Slusarz

gcc warns about it:
sound/core/pcm_native.c: In function 'snd_pcm_fasync':
sound/core/pcm_native.c:3262: warning: label 'out' defined but not used

Signed-off-by Marcin Slusarz <[email protected]>
Cc Takashi Iwai <[email protected]>
Cc Jaroslav Kysela <[email protected]>
---
 sound/core/pcm_native.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
sound/core/pcm_native.c +0 -0
index c49b9d9..333cff6 100644

sound/core/pcm_native.c static int snd_pcm_fasync(int fd, struct file * file, int on) · line 3264

Quoted 7 lines
sound/core/pcm_native.c +0 -1
@@ -3259,7 +3259,6 @@ static int snd_pcm_fasync(int fd, struct file * file, int on)
 	runtime = substream->runtime;
 
 	err = fasync_helper(fd, file, on, &runtime->fasync);
-out:
 	unlock_kernel();
 	if (err < 0)
 		return err;

Uhm, no, there's

snd_assert(substream != NULL, goto out);

one line above your context. The snd_assert() macro will only include the second argument with CONFIG_SND_DEBUG, without CONFIG_SND_DEBUG it just evaulates the first parameter and triggers the warning.

Sven

#2 lore raw
Quoted 28 lines · Sven Wegener
Quoted 16 lines · Marcin Slusarz

gcc warns about it:
sound/core/pcm_native.c: In function 'snd_pcm_fasync':
sound/core/pcm_native.c:3262: warning: label 'out' defined but not used

Signed-off-by Marcin Slusarz <[email protected]>
Cc Takashi Iwai <[email protected]>
Cc Jaroslav Kysela <[email protected]>
---
 sound/core/pcm_native.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
sound/core/pcm_native.c +0 -0
index c49b9d9..333cff6 100644

sound/core/pcm_native.c static int snd_pcm_fasync(int fd, struct file * file, int on) · line 3264

Quoted 7 lines
sound/core/pcm_native.c +0 -1
@@ -3259,7 +3259,6 @@ static int snd_pcm_fasync(int fd, struct file * file, int on)
 	runtime = substream->runtime;
 
 	err = fasync_helper(fd, file, on, &runtime->fasync);
-out:
 	unlock_kernel();
 	if (err < 0)
 		return err;

Uhm, no, there's

Quoted 3 lines

snd_assert(substream != NULL, goto out);

one line above your context.

And that is broken. That's not what an assert is for, we access substream->runtime a line later and that will blow up. That should be a simpe if, instead of an assert.

Sven

author
#3 lore raw
Quoted 30 lines · Sven Wegener
Quoted 28 lines · Sven Wegener
Quoted 16 lines · Marcin Slusarz

gcc warns about it:
sound/core/pcm_native.c: In function 'snd_pcm_fasync':
sound/core/pcm_native.c:3262: warning: label 'out' defined but not used

Signed-off-by Marcin Slusarz <[email protected]>
Cc Takashi Iwai <[email protected]>
Cc Jaroslav Kysela <[email protected]>
---
 sound/core/pcm_native.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
sound/core/pcm_native.c +0 -0
index c49b9d9..333cff6 100644

sound/core/pcm_native.c static int snd_pcm_fasync(int fd, struct file * file, int on) · line 3264

Quoted 7 lines
sound/core/pcm_native.c +0 -1
@@ -3259,7 +3259,6 @@ static int snd_pcm_fasync(int fd, struct file * file, int on)
 	runtime = substream->runtime;
 
 	err = fasync_helper(fd, file, on, &runtime->fasync);
-out:
 	unlock_kernel();
 	if (err < 0)
 		return err;

Uhm, no, there's

Quoted 3 lines
Quoted 3 lines

snd_assert(substream != NULL, goto out);

one line above your context.

Brown paper bag for me, please.
But for my defense, here's the code:

static int snd_pcm_fasync(int fd, struct file * file, int on)

{
	struct snd_pcm_file * pcm_file;
	struct snd_pcm_substream *substream;
	struct snd_pcm_runtime *runtime;
	int err = -ENXIO;

	lock_kernel();
	pcm_file = file->private_data;
	substream = pcm_file->substream;
	snd_assert(substream != NULL, goto out);
	runtime = substream->runtime;

	err = fasync_helper(fd, file, on, &runtime->fasync);
out:
	unlock_kernel();
	if (err < 0)
		return err;
	return 0;
}

It's a bit weird to have constructs like snd_assert which look like function call, but actually changes program flow...

Quoted 3 lines

And that is broken. That's not what an assert is for, we access substream->runtime a line later and that will blow up. That should be a simpe if, instead of an assert.

I would say this whole snd_assert macro is broken...

$ git grep snd_assert|wc -l
829
$ git grep snd_assert|grep return|wc -l
722
$ git grep snd_assert|grep return|grep NULL|wc -l
381

Marcin