| Message ID | 20180320151857.26340-1-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | chardev-socket: remove useless if | expand |
On Tue, Mar 20, 2018 at 4:18 PM, Paolo Bonzini <pbonzini@redhat.com> 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 <pbonzini@redhat.com> > --- Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > 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 > >
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 <pbonzini@redhat.com> (maybe replacing with an assertion would be nicer? No big deal.) Reviewed-by: Peter Xu <peterx@redhat.com>
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 <pbonzini@redhat.com> > > (maybe replacing with an assertion would be nicer? No big deal.) It's already asserting, it just raises SIGSEGV instead of SIGABRT. :) Paolo
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 <pbonzini@redhat.com> > > > > (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(). :)
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,
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 <pbonzini@redhat.com> --- chardev/char-socket.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)