[Qemu-devel] [PATCH] vnc: fix coverity warning

[Qemu-devel] [PATCH] vnc: fix coverity warning

[Gerd Hoffmann]

vnc_display_local_addr will not be called with an invalid display id.
Add assert() to silence coverity warning about a null pointer dereference.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/vnc.c b/ui/vnc.c
index e291987..25ba340 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3252,6 +3252,7 @@ char *vnc_display_local_addr(const char *id)
 {
     VncDisplay *vs = vnc_display_find(id);
 
+    assert(vs);
     return vnc_socket_local_addr("%s:%s", vs->lsock);
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] vnc: fix coverity warning

[Paolo Bonzini]

On 11/02/2015 17:37, Gerd Hoffmann wrote:
> vnc_display_local_addr will not be called with an invalid display id.
> Add assert() to silence coverity warning about a null pointer dereference.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/vnc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index e291987..25ba340 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3252,6 +3252,7 @@ char *vnc_display_local_addr(const char *id)
>  {
>      VncDisplay *vs = vnc_display_find(id);
>  
> +    assert(vs);
>      return vnc_socket_local_addr("%s:%s", vs->lsock);
>  }
>  
> 

If you want, I can just silence the warning in Coverity.

Paolo

Re: [Qemu-devel] [PATCH] vnc: fix coverity warning

[Gerd Hoffmann]

> > +    assert(vs);
> >      return vnc_socket_local_addr("%s:%s", vs->lsock);
> >  }
> >  
> > 
> 
> If you want, I can just silence the warning in Coverity.

I think in the source code is better, human readers might wonder too. 

And in case the calling code ever changes behavior the assert will trap
it (even though it is highly unlikely in this specific case).

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] vnc: fix coverity warning

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

>> > +    assert(vs);
>> >      return vnc_socket_local_addr("%s:%s", vs->lsock);
>> >  }
>> >  
>> > 
>> 
>> If you want, I can just silence the warning in Coverity.
>
> I think in the source code is better, human readers might wonder too. 
>
> And in case the calling code ever changes behavior the assert will trap
> it (even though it is highly unlikely in this specific case).

I'm very much in favor of using assertions to guide Coverity.  Besides
human readers, they may also help other static analyzers, including
optimizers.

Speaking of silencing defects: we could also explore use of code
annotations.  Example given in the manual:

    x = NULL;
    ...
    // coverity[var_deref_op]
    *x = 0;

The comment automatically classifies the FORWARD_NULL as intentional.