[libvirt PATCH 06/11] qemu_monitor_json: explicitly ignore return values

Pavel Hrdina posted 11 patches 5 years, 2 months ago
[libvirt PATCH 06/11] qemu_monitor_json: explicitly ignore return values
Posted by Pavel Hrdina 5 years, 2 months ago
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/qemu/qemu_monitor_json.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 723bdb4426..51730a5fe3 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1370,8 +1370,8 @@ qemuMonitorJSONHandleMemoryFailure(qemuMonitorPtr mon,
     }
 
     if (flagsjson) {
-        virJSONValueObjectGetBoolean(flagsjson, "action-required", &ar);
-        virJSONValueObjectGetBoolean(flagsjson, "recursive", &recursive);
+        ignore_value(virJSONValueObjectGetBoolean(flagsjson, "action-required", &ar));
+        ignore_value(virJSONValueObjectGetBoolean(flagsjson, "recursive", &recursive));
     }
 
     mf.recipient = recipient;
-- 
2.26.2

Re: [libvirt PATCH 06/11] qemu_monitor_json: explicitly ignore return values
Posted by Peter Krempa 5 years, 2 months ago
On Mon, Nov 16, 2020 at 16:38:53 +0100, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/qemu/qemu_monitor_json.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

I presume this silences a coverity moan about "most of the callers check
the value". NACK in that case.

Re: [libvirt PATCH 06/11] qemu_monitor_json: explicitly ignore return values
Posted by Pavel Hrdina 5 years, 2 months ago
On Mon, Nov 16, 2020 at 04:58:56PM +0100, Peter Krempa wrote:
> On Mon, Nov 16, 2020 at 16:38:53 +0100, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
> >  src/qemu/qemu_monitor_json.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> I presume this silences a coverity moan about "most of the callers check
> the value". NACK in that case.

Yes it does. In that case we should remove ignore_value() from the
remaining calls to that function that are not checked.

Pavel
Re: [libvirt PATCH 06/11] qemu_monitor_json: explicitly ignore return values
Posted by Peter Krempa 5 years, 2 months ago
On Mon, Nov 16, 2020 at 17:09:17 +0100, Pavel Hrdina wrote:
> On Mon, Nov 16, 2020 at 04:58:56PM +0100, Peter Krempa wrote:
> > On Mon, Nov 16, 2020 at 16:38:53 +0100, Pavel Hrdina wrote:
> > > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > > ---
> > >  src/qemu/qemu_monitor_json.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > I presume this silences a coverity moan about "most of the callers check
> > the value". NACK in that case.
> 
> Yes it does. In that case we should remove ignore_value() from the
> remaining calls to that function that are not checked.

Either that, or for a full fix add G_GNUC_UNUSED to the function header.
Without that nothing would be really fixed and noting will be prevented
in the future.

Re: [libvirt PATCH 06/11] qemu_monitor_json: explicitly ignore return values
Posted by Pavel Hrdina 5 years, 2 months ago
On Mon, Nov 16, 2020 at 05:20:00PM +0100, Peter Krempa wrote:
> On Mon, Nov 16, 2020 at 17:09:17 +0100, Pavel Hrdina wrote:
> > On Mon, Nov 16, 2020 at 04:58:56PM +0100, Peter Krempa wrote:
> > > On Mon, Nov 16, 2020 at 16:38:53 +0100, Pavel Hrdina wrote:
> > > > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > > > ---
> > > >  src/qemu/qemu_monitor_json.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > I presume this silences a coverity moan about "most of the callers check
> > > the value". NACK in that case.
> > 
> > Yes it does. In that case we should remove ignore_value() from the
> > remaining calls to that function that are not checked.
> 
> Either that, or for a full fix add G_GNUC_UNUSED to the function header.
> Without that nothing would be really fixed and noting will be prevented
> in the future.

I guess you mean using G_GNUC_WARN_UNUSED_RESULT.

There are other cases where coverity complains that the return value is
not checked but probably not all cases, only where the majority is
checked.

I'll ignore these issues for now.

Pavel
Re: [libvirt PATCH 06/11] qemu_monitor_json: explicitly ignore return values
Posted by Peter Krempa 5 years, 2 months ago
On Mon, Nov 16, 2020 at 18:04:38 +0100, Pavel Hrdina wrote:
> On Mon, Nov 16, 2020 at 05:20:00PM +0100, Peter Krempa wrote:
> > On Mon, Nov 16, 2020 at 17:09:17 +0100, Pavel Hrdina wrote:
> > > On Mon, Nov 16, 2020 at 04:58:56PM +0100, Peter Krempa wrote:
> > > > On Mon, Nov 16, 2020 at 16:38:53 +0100, Pavel Hrdina wrote:
> > > > > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > > > > ---
> > > > >  src/qemu/qemu_monitor_json.c | 4 ++--
> > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > I presume this silences a coverity moan about "most of the callers check
> > > > the value". NACK in that case.
> > > 
> > > Yes it does. In that case we should remove ignore_value() from the
> > > remaining calls to that function that are not checked.
> > 
> > Either that, or for a full fix add G_GNUC_UNUSED to the function header.
> > Without that nothing would be really fixed and noting will be prevented
> > in the future.
> 
> I guess you mean using G_GNUC_WARN_UNUSED_RESULT.

Yes, that one, sorry.

> There are other cases where coverity complains that the return value is
> not checked but probably not all cases, only where the majority is
> checked.

Yeah, this heuristic is of questionable value. I can see that it might
find "some" cases of problems, but the annoying part is that it can
appear randomly after adding a totally irrelevant piece of code.

I'd specifically not address this kind of problems unless there's a real
bug.