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(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index c49b9d9..333cff6 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -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;
--
1.5.4.5
On Sun, 3 Aug 2008, Marcin Slusarz wrote:
> 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(-)
>
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index c49b9d9..333cff6 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -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
On Sun, 3 Aug 2008, Sven Wegener wrote:
> On Sun, 3 Aug 2008, Marcin Slusarz wrote:
>
> > 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(-)
> >
> > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> > index c49b9d9..333cff6 100644
> > --- a/sound/core/pcm_native.c
> > +++ b/sound/core/pcm_native.c
> > @@ -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.
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
On Sun, Aug 03, 2008 at 07:33:19PM +0200, Sven Wegener wrote:
> On Sun, 3 Aug 2008, Sven Wegener wrote:
>
> > On Sun, 3 Aug 2008, Marcin Slusarz wrote:
> >
> > > 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(-)
> > >
> > > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> > > index c49b9d9..333cff6 100644
> > > --- a/sound/core/pcm_native.c
> > > +++ b/sound/core/pcm_native.c
> > > @@ -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.
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...
> 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