| Message ID | 20231109162034.2108018-1-manos.pitsidianakis@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-snd: check AUD_register_card return value | expand |
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 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, 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.
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
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;
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(-) base-commit: ad6ef0a42e314a8c6ac6c96d5f6e607a1e5644b5