Amit Shah <amit.s...@redhat.com> writes: > On (Mon) 17 Dec 2012 [14:14:17], Markus Armbruster wrote: >> Amit Shah <amit.s...@redhat.com> writes: >> >> > Stuff the cpkt before calling send_control_msg(). it should not be >> > concerned about contents, just send across the buffer. >> > >> > A few code refactorings recently have made mkaing this change easier >> > than earlier. > > Ugh, I'll fix the typo and incoherent language here before merging. > >> > Coverity and clang have flagged this code several times in the past >> > (cpkt->id not set before send_control_event() passed it on to >> > send_control_msg()). This will finally eliminate the false-positive. >> > >> > CC: Markus Armbruster <arm...@redhat.com> >> > Signed-off-by: Amit Shah <amit.s...@redhat.com> >> >> Patch makes sense to me, and the Coverity defect report is gone. > > Thanks for checking! > >> However, it now worries find_port_by_id() in remove_port() could return >> a null pointer, which is then dereferenced. No idea why it didn't >> report that before. Obvious suppressor: >> >> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c >> index 47d0481..7ff7505 100644 >> --- a/hw/virtio-serial-bus.c >> +++ b/hw/virtio-serial-bus.c >> @@ -826,6 +826,7 @@ static void remove_port(VirtIOSerial *vser, uint32_t >> port_id) >> vser->ports_map[i] &= ~(1U << (port_id % 32)); >> >> port = find_port_by_id(vser, port_id); >> + assert(port); >> /* Flush out any unconsumed buffers first */ >> discard_vq_data(port->ovq, &port->vser->vdev); > > remove_port() is called by the hot-unplug qdev callback, and if the > port's missing from our tailq, something's gone wrong anyway. So this > patch makes sense too.
Will you take care of that, or do you want me to post the patch?
