[Qemu-devel] [PATCH] chardev-socket: remove useless if

[Paolo Bonzini]

This trips Coverity, which believes the subsequent qio_channel_create_watch
can dereference a NULL pointer.  In reality, tcp_chr_connect's callers
all have s->ioc properly initialized, since they are all rooted at
tcp_chr_new_client.

Signed-off-by: Paolo Bonzini 
---
 chardev/char-socket.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index d057192ced..159e69c3b1 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -550,12 +550,10 @@ static void tcp_chr_connect(void *opaque)
 s->is_listen, s->is_telnet);
 
 s->connected = 1;
-if (s->ioc) {
-chr->gsource = io_add_watch_poll(chr, s->ioc,
-   tcp_chr_read_poll,
-   tcp_chr_read,
-   chr, chr->gcontext);
-}
+chr->gsource = io_add_watch_poll(chr, s->ioc,
+   tcp_chr_read_poll,
+   tcp_chr_read,
+   chr, chr->gcontext);
 
 s->hup_source = qio_channel_create_watch(s->ioc, G_IO_HUP);
 g_source_set_callback(s->hup_source, (GSourceFunc)tcp_chr_hup,
-- 
2.16.2

Re: [Qemu-devel] [PATCH] chardev-socket: remove useless if

[Marc-André Lureau]

On Tue, Mar 20, 2018 at 4:18 PM, Paolo Bonzini  wrote:
> This trips Coverity, which believes the subsequent qio_channel_create_watch
> can dereference a NULL pointer.  In reality, tcp_chr_connect's callers
> all have s->ioc properly initialized, since they are all rooted at
> tcp_chr_new_client.
>
> Signed-off-by: Paolo Bonzini 
> ---

Reviewed-by: Marc-André Lureau 


>  chardev/char-socket.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index d057192ced..159e69c3b1 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -550,12 +550,10 @@ static void tcp_chr_connect(void *opaque)
>  s->is_listen, s->is_telnet);
>
>  s->connected = 1;
> -if (s->ioc) {
> -chr->gsource = io_add_watch_poll(chr, s->ioc,
> -   tcp_chr_read_poll,
> -   tcp_chr_read,
> -   chr, chr->gcontext);
> -}
> +chr->gsource = io_add_watch_poll(chr, s->ioc,
> +   tcp_chr_read_poll,
> +   tcp_chr_read,
> +   chr, chr->gcontext);
>
>  s->hup_source = qio_channel_create_watch(s->ioc, G_IO_HUP);
>  g_source_set_callback(s->hup_source, (GSourceFunc)tcp_chr_hup,
> --
> 2.16.2
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH] chardev-socket: remove useless if

[Peter Xu]

On Tue, Mar 20, 2018 at 04:18:57PM +0100, Paolo Bonzini wrote:
> This trips Coverity, which believes the subsequent qio_channel_create_watch
> can dereference a NULL pointer.  In reality, tcp_chr_connect's callers
> all have s->ioc properly initialized, since they are all rooted at
> tcp_chr_new_client.
> 
> Signed-off-by: Paolo Bonzini 

(maybe replacing with an assertion would be nicer? No big deal.)

Reviewed-by: Peter Xu 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH] chardev-socket: remove useless if

[Paolo Bonzini]

On 21/03/2018 04:25, Peter Xu wrote:
> On Tue, Mar 20, 2018 at 04:18:57PM +0100, Paolo Bonzini wrote:
>> This trips Coverity, which believes the subsequent qio_channel_create_watch
>> can dereference a NULL pointer.  In reality, tcp_chr_connect's callers
>> all have s->ioc properly initialized, since they are all rooted at
>> tcp_chr_new_client.
>>
>> Signed-off-by: Paolo Bonzini 
> 
> (maybe replacing with an assertion would be nicer? No big deal.)

It's already asserting, it just raises SIGSEGV instead of SIGABRT. :)

Paolo

Re: [Qemu-devel] [PATCH] chardev-socket: remove useless if

[Peter Xu]

On Wed, Mar 21, 2018 at 06:50:46AM +0100, Paolo Bonzini wrote:
> On 21/03/2018 04:25, Peter Xu wrote:
> > On Tue, Mar 20, 2018 at 04:18:57PM +0100, Paolo Bonzini wrote:
> >> This trips Coverity, which believes the subsequent qio_channel_create_watch
> >> can dereference a NULL pointer.  In reality, tcp_chr_connect's callers
> >> all have s->ioc properly initialized, since they are all rooted at
> >> tcp_chr_new_client.
> >>
> >> Signed-off-by: Paolo Bonzini 
> > 
> > (maybe replacing with an assertion would be nicer? No big deal.)
> 
> It's already asserting, it just raises SIGSEGV instead of SIGABRT. :)

Ah yes it's in the next qio_channel_create_watch(). :)

-- 
Peter Xu