Re: [PATCH 1/1] Clean redundant code about VCPU string checking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/18/2013 05:57 AM, Li Zhang wrote:
> From: Li Zhang <zhlcindy@xxxxxxxxxxxxxxxxxx>
> 
> Now that VCPU number are removed from qemu_monitor_text.c.
> VCPU string checking also should be removed.
> 
> Report-by: John Ferlan <jferlan@xxxxxxxxxx>
> Signed-off-by: Li Zhang <zhlcindy@xxxxxxxxxxxxxxxxxx>
> ---
>  src/qemu/qemu_monitor_text.c |    9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)

>From the view point of this fix resolves the Coverity complaint/error,
this seems fine. However, one nit I saw when looking at the code...

The line:

        VIR_DEBUG("pid=%d", tid);


probably should be

        VIR_DEBUG("tid=%d", tid);


> 
> diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
> index 1b6efba..3a0c55f 100644
> --- a/src/qemu/qemu_monitor_text.c
> +++ b/src/qemu/qemu_monitor_text.c
> @@ -527,17 +527,10 @@ int qemuMonitorTextGetCPUInfo(qemuMonitorPtr mon,
>       */
>      line = qemucpus;
>      do {
> -        char *offset = strchr(line, '#');
> +        char *offset = NULL;
>          char *end = NULL;
>          int tid = 0;
>  
> -        /* See if we're all done */
> -        if (offset == NULL)
> -            break;

I'm not familiar with the output, but if a line didn't have the '#' in
it is there any reason not to break?  Especially since now if it doesn't
have thread_id in it, we're going to go to error?

John

> -
> -        if (end == NULL || *end != ':')
> -            goto error;
> -
>          /* Extract host Thread ID */
>          if ((offset = strstr(line, "thread_id=")) == NULL)
>              goto error;
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]