AUD_register_card might fail. Even though errp was passed as an
argument, the call's return value was not checked for failure.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 hw/audio/virtio-snd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index a18a9949a7..ccf5fcf99e 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -1102,7 +1102,9 @@ static void virtio_snd_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    AUD_register_card("virtio-sound", &vsnd->card, errp);
+    if (!AUD_register_card("virtio-sound", &vsnd->card, errp)) {
+        return;
+    }
 
     /* set default params for all streams */
     default_params.features = 0;

base-commit: ad6ef0a42e314a8c6ac6c96d5f6e607a1e5644b5
-- 
2.39.2

On 9/11/23 17:20, Manos Pitsidianakis wrote:
> AUD_register_card might fail. Even though errp was passed as an
> argument, the call's return value was not checked for failure.
> 
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
>   hw/audio/virtio-snd.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

On Thu, 9 Nov 2023 at 16:21, Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
>
> AUD_register_card might fail. Even though errp was passed as an
> argument, the call's return value was not checked for failure.

For whoever picks up this patch: we can add
"Fixes Coverity CID 1523899" to the commit message.

> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
>  hw/audio/virtio-snd.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> index a18a9949a7..ccf5fcf99e 100644
> --- a/hw/audio/virtio-snd.c
> +++ b/hw/audio/virtio-snd.c
> @@ -1102,7 +1102,9 @@ static void virtio_snd_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>
> -    AUD_register_card("virtio-sound", &vsnd->card, errp);
> +    if (!AUD_register_card("virtio-sound", &vsnd->card, errp)) {
> +        return;
> +    }
>
>      /* set default params for all streams */
>      default_params.features = 0;

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

On Thu, Nov 09, 2023 at 04:25:04PM +0000, Peter Maydell wrote:
> On Thu, 9 Nov 2023 at 16:21, Manos Pitsidianakis
> <manos.pitsidianakis@linaro.org> wrote:
> >
> > AUD_register_card might fail. Even though errp was passed as an
> > argument, the call's return value was not checked for failure.
> 
> For whoever picks up this patch: we can add
> "Fixes Coverity CID 1523899" to the commit message.


Better:

Fixes: Coverity CID 1523899

> > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > ---
> >  hw/audio/virtio-snd.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> > index a18a9949a7..ccf5fcf99e 100644
> > --- a/hw/audio/virtio-snd.c
> > +++ b/hw/audio/virtio-snd.c
> > @@ -1102,7 +1102,9 @@ static void virtio_snd_realize(DeviceState *dev, Error **errp)
> >          return;
> >      }
> >
> > -    AUD_register_card("virtio-sound", &vsnd->card, errp);
> > +    if (!AUD_register_card("virtio-sound", &vsnd->card, errp)) {
> > +        return;
> > +    }
> >
> >      /* set default params for all streams */
> >      default_params.features = 0;
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> thanks
> -- PMM

On Thu, 9 Nov 2023 at 17:53, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, Nov 09, 2023 at 04:25:04PM +0000, Peter Maydell wrote:
> > On Thu, 9 Nov 2023 at 16:21, Manos Pitsidianakis
> > <manos.pitsidianakis@linaro.org> wrote:
> > >
> > > AUD_register_card might fail. Even though errp was passed as an
> > > argument, the call's return value was not checked for failure.
> >
> > For whoever picks up this patch: we can add
> > "Fixes Coverity CID 1523899" to the commit message.
>
>
> Better:
>
> Fixes: Coverity CID 1523899

I thought "Fixes:" as a header-line like that was for
the commit hash/subject of the commit the patch is fixing?

thanks
-- PMM

On Thu, Nov 09, 2023 at 06:03:15PM +0000, Peter Maydell wrote:
> On Thu, 9 Nov 2023 at 17:53, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Thu, Nov 09, 2023 at 04:25:04PM +0000, Peter Maydell wrote:
> > > On Thu, 9 Nov 2023 at 16:21, Manos Pitsidianakis
> > > <manos.pitsidianakis@linaro.org> wrote:
> > > >
> > > > AUD_register_card might fail. Even though errp was passed as an
> > > > argument, the call's return value was not checked for failure.
> > >
> > > For whoever picks up this patch: we can add
> > > "Fixes Coverity CID 1523899" to the commit message.
> >
> >
> > Better:
> >
> > Fixes: Coverity CID 1523899
> 
> I thought "Fixes:" as a header-line like that was for
> the commit hash/subject of the commit the patch is fixing?
> 
> thanks
> -- PMM

This works for many other things
e.g. gitlab issues (closes them). Fixes without : is much harder to
distinguish from just general english text.
qemu uses a mix of Fixes: Resolves: and Closes: .
I don't see a real need for distinct tags for commit versus gitlab
issue link: one can look at the contents to figure that out.

-- 
MST

On Fri, 10 Nov 2023 01:23, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>On Thu, Nov 09, 2023 at 06:03:15PM +0000, Peter Maydell wrote:
>> On Thu, 9 Nov 2023 at 17:53, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >
>> > On Thu, Nov 09, 2023 at 04:25:04PM +0000, Peter Maydell wrote:
>> > > On Thu, 9 Nov 2023 at 16:21, Manos Pitsidianakis
>> > > <manos.pitsidianakis@linaro.org> wrote:
>> > > >
>> > > > AUD_register_card might fail. Even though errp was passed as an
>> > > > argument, the call's return value was not checked for failure.
>> > >
>> > > For whoever picks up this patch: we can add
>> > > "Fixes Coverity CID 1523899" to the commit message.
>> >
>> >
>> > Better:
>> >
>> > Fixes: Coverity CID 1523899
>> 
>> I thought "Fixes:" as a header-line like that was for
>> the commit hash/subject of the commit the patch is fixing?
>> 
>> thanks
>> -- PMM
>
>This works for many other things
>e.g. gitlab issues (closes them). Fixes without : is much harder to
>distinguish from just general english text.
>qemu uses a mix of Fixes: Resolves: and Closes: .
>I don't see a real need for distinct tags for commit versus gitlab
>issue link: one can look at the contents to figure that out.

The "Fixes:" trailer is for commits.
In the kernel they use "Addresses-Coverity-ID: ..." but I can't find out 
if it's part of some automated workflow or just convention.

Example commit in torvalds/linux: 
5ad2e46030ad97de7fdbdaf63bb1af45c7caf3dd