[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] svm: rephrase local variable use for Coverity.

On 01/06/2016 08:24 AM, Jan Beulich wrote:
On 01.01.16 at 04:14, <jtotto@xxxxxxxxxxxx> wrote:
Coverity CID 1343310

No functional changes.

Signed-off-by: Joshua Otto <jtotto@xxxxxxxxxxxx>
On Mon, Dec 28, 2015 at 09:34:28AM +0000, Andrew Cooper wrote:
The error message isn't fantastic, but the complaint that Coverity
has is that we store intr here, then unilaterally store it again
slightly lower in the function, no matter what value it had (with
the early return presumably not being taken into account).

The error would probably be resolved if lines 95 and 96 turned into
"if ( vmcb_get_vintr(gvmcb).fields.irq )"
This patch implements that change - as a general rule, is maintainer
preference to resolve false positives like this by suppressing them in
the tool or through code changes like this one?

I'd rather suppress this in the tool as I am one of those people that Jan refers to below ;-)

However, if it's too much of a hassle then this patch would be OK.


Asking such a question it would be helpful if you included the
maintainers of the code in question, since to a good part this
is a matter of taste, especially when ...

--- a/xen/arch/x86/hvm/svm/intr.c
+++ b/xen/arch/x86/hvm/svm/intr.c
@@ -92,8 +92,7 @@ static void svm_enable_intr_window(struct vcpu *v, struct 
hvm_intack intack)
               * return here or l2 guest looses interrupts, otherwise.
              ASSERT(gvmcb != NULL);
-            intr = vmcb_get_vintr(gvmcb);
-            if ( intr.fields.irq )
+            if ( vmcb_get_vintr(gvmcb).fields.irq )
... some people (not me) frown upon complex expressions like the
one resulting here.

Also please note that while perhaps minor here, obvious with
the quote from an earlier mail conversation, naming the person
having suggested the change would be appropriate - if you
look for them, you'll find quite a few Suggested-by: tags in the
commit history.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.