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

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



On 2013年03月19日 01:01, John Ferlan wrote:
On 03/18/2013 05:57 AM, Li Zhang wrote:
From: Li Zhang <zhlcindy linux vnet ibm com>

Now that VCPU number are removed from qemu_monitor_text.c.
VCPU string checking also should be removed.

Report-by: John Ferlan <jferlan redhat com>
Signed-off-by: Li Zhang <zhlcindy linux vnet ibm com>
---
  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);

From code, tid is stored to cpupids.
I think pid should be fine here. :)


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?
(qemu) info cpus
* CPU #0: nip=0x000000000dcd8814 thread_id=49285
CPU #4: nip=0x0000000000000100 (halted) thread_id=49286
CPU #8: nip=0x0000000000000100 (halted) thread_id=49287
CPU #12: nip=0x0000000000000100 (halted) thread_id=49288

From QEMU's code, there is one '\r\n' by the end of every line.
Have a second thought, the last position of line will be at \r or \n.
It won't be out of loop unless when thread_id can't be found.

Ah, it seems that it's necessary to keep the break.

Thanks.

-
-        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 redhat com
https://www.redhat.com/mailman/listinfo/libvir-list


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