On Thu, Nov 05, 2015 at 03:18:12PM -0500, John Ferlan wrote:
>
>
> On 11/05/2015 12:33 PM, Daniel P. Berrange wrote:
> > As of QEMU 0.11.0 the 'info chardev' monitor command can be
> > used to report on allocated chardev paths, so we can drop
> > support for parsing QEMU stderr to locate the PTY paths.
> >
> > Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> > ---
> > src/qemu/qemu_process.c | 193 ------------------------------------------------
> > 1 file changed, 193 deletions(-)
> >
>
> Ran the series through the Coverity checks...
>
> > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> > index 524072c..72c7041 100644
> > --- a/src/qemu/qemu_process.c
> > +++ b/src/qemu/qemu_process.c
>
> [...]
>
> > static int
> > qemuProcessRefreshChannelVirtioState(virQEMUDriverPtr driver,
> > @@ -2140,15 +1956,6 @@ qemuProcessWaitForMonitor(virQEMUDriverPtr driver,
> > (logfd = qemuDomainOpenLog(driver, vm, pos)) < 0)
> > return -1;
> >
> > - if (logfd != -1 && !virQEMUCapsUsedQMP(qemuCaps)) {
> > - if (VIR_ALLOC_N(buf, buf_size) < 0)
> > - goto closelog;
> > -
> > - if (qemuProcessReadLogOutput(vm, logfd, buf, buf_size,
> > - qemuProcessFindCharDevicePTYs,
> > - "console", 30) < 0)
> > - goto closelog;
> > - }
> >
> > VIR_DEBUG("Connect monitor to %p '%s'", vm, vm->def->name);
> > if (qemuConnectMonitor(driver, vm, asyncJob, logfd) < 0)
> >
>
> Complaint from Coverity is in the cleanup: section
>
>
> (11) Event cond_false: Condition "virQEMUCapsUsedQMP(qemuCaps)", taking
> false branch
>
> 1993 if (virQEMUCapsUsedQMP(qemuCaps)) {
> 1994 if (VIR_ALLOC_N(buf, buf_size) < 0)
> 1995 goto closelog;
>
> (12) Event if_end: End of if statement
>
> 1996 }
> 1997
>
> (13) Event var_deref_model: Passing null pointer "buf" to "strlen",
> which dereferences it.
> Also see events: [assign_zero]
>
>
> So if virQEMUCapsUsedQMP fails, then buf is NULL
Ok, we need to unconditionally allocate buf, and get ridof the strlen
call. So I'm adding this patch
@@ -1987,20 +1987,16 @@ qemuProcessWaitForMonitor(virQEMUDriverPtr driver,
virHashFree(info);
if (pos != -1 && kill(vm->pid, 0) == -1 && errno == ESRCH) {
- int len;
/* VM is dead, any other error raised in the interim is probably
* not as important as the qemu cmdline output */
- if (virQEMUCapsUsedQMP(qemuCaps)) {
- if (VIR_ALLOC_N(buf, buf_size) < 0)
- goto closelog;
- }
+ if (VIR_ALLOC_N(buf, buf_size) < 0)
+ goto closelog;
- len = strlen(buf);
/* best effort seek - we need to reset to the original position, so that
* a possible read of the fd in the monitor code doesn't influence this
* error delivery option */
ignore_value(lseek(logfd, pos, SEEK_SET));
- qemuProcessReadLog(logfd, buf + len, buf_size - len - 1, 0, true);
+ qemuProcessReadLog(logfd, buf, buf_size - 1, 0, true);
virReportError(VIR_ERR_INTERNAL_ERROR,
_("process exited while connecting to monitor: %s"),
buf);
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list