Re: [libvirt] [PATCH] qemu: fix text block info parsing

2011-10-13 Thread Daniel Veillard
On Wed, Oct 12, 2011 at 06:30:08PM -0600, Eric Blake wrote:
 Detected by Coverity.  p (the pointer to the string) is always true;
 when in reality, we wanted to know whether the integer value of the
 just-parsed string is '0' or '1'.  Logic bug since commit b1b5b51.
 
 * src/qemu/qemu_monitor_text.c (qemuMonitorTextGetBlockInfo): Set
 results to proper value.
 ---
 
 Second bug introduced by the same commit.  And I think I reviewed
 it originally.  Sorry for not catching it earlier.

  Absolutely no need to be sorry, 1/ you're the one ACK'ing the most
patches 2/ I think you filter out bugs way better than me for example
statistics will play against you, but nobody will complain !

  src/qemu/qemu_monitor_text.c |6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
 index 1eb9846..49c469b 100644
 --- a/src/qemu/qemu_monitor_text.c
 +++ b/src/qemu/qemu_monitor_text.c
 @@ -817,19 +817,19 @@ int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon,
  if (virStrToLong_i(p, dummy, 10, tmp) == -1)
  VIR_DEBUG(error reading removable: %s, p);
  else
 -info-removable = p != NULL;
 +info-removable = tmp != 0;

  I would be tempted to put braces 
   info-removable = (tmp != 0);
to make it obvious that the second part is computation, not cascaded
affectations 

  } else if (STRPREFIX(p, locked=)) {
  p += strlen(locked=);
  if (virStrToLong_i(p, dummy, 10, tmp) == -1)
  VIR_DEBUG(error reading locked: %s, p);
  else
 -info-locked = p ? true : false;
 +info-locked = tmp != 0;
  } else if (STRPREFIX(p, tray_open=)) {
  p += strlen(tray_open=);
  if (virStrToLong_i(p, dummy, 10, tmp) == -1)
  VIR_DEBUG(error reading tray_open: %s, p);
  else
 -info-tray_open = p ? true : false;
 +info-tray_open = tmp != 0;
  } else {
  /* ignore because we don't parse all options */
  }

  But it's cosmetic, ACK

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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


Re: [libvirt] [PATCH] qemu: fix text block info parsing

2011-10-13 Thread Eric Blake

On 10/13/2011 03:15 AM, Daniel Veillard wrote:

On Wed, Oct 12, 2011 at 06:30:08PM -0600, Eric Blake wrote:

Detected by Coverity.  p (the pointer to the string) is always true;
when in reality, we wanted to know whether the integer value of the
just-parsed string is '0' or '1'.  Logic bug since commit b1b5b51.

* src/qemu/qemu_monitor_text.c (qemuMonitorTextGetBlockInfo): Set
results to proper value.
---




-info-removable = p != NULL;
+info-removable = tmp != 0;


   I would be tempted to put braces
info-removable = (tmp != 0);
to make it obvious that the second part is computation, not cascaded
affectations


Done,



   But it's cosmetic, ACK


and pushed.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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


[libvirt] [PATCH] qemu: fix text block info parsing

2011-10-12 Thread Eric Blake
Detected by Coverity.  p (the pointer to the string) is always true;
when in reality, we wanted to know whether the integer value of the
just-parsed string is '0' or '1'.  Logic bug since commit b1b5b51.

* src/qemu/qemu_monitor_text.c (qemuMonitorTextGetBlockInfo): Set
results to proper value.
---

Second bug introduced by the same commit.  And I think I reviewed
it originally.  Sorry for not catching it earlier.

 src/qemu/qemu_monitor_text.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 1eb9846..49c469b 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -817,19 +817,19 @@ int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon,
 if (virStrToLong_i(p, dummy, 10, tmp) == -1)
 VIR_DEBUG(error reading removable: %s, p);
 else
-info-removable = p != NULL;
+info-removable = tmp != 0;
 } else if (STRPREFIX(p, locked=)) {
 p += strlen(locked=);
 if (virStrToLong_i(p, dummy, 10, tmp) == -1)
 VIR_DEBUG(error reading locked: %s, p);
 else
-info-locked = p ? true : false;
+info-locked = tmp != 0;
 } else if (STRPREFIX(p, tray_open=)) {
 p += strlen(tray_open=);
 if (virStrToLong_i(p, dummy, 10, tmp) == -1)
 VIR_DEBUG(error reading tray_open: %s, p);
 else
-info-tray_open = p ? true : false;
+info-tray_open = tmp != 0;
 } else {
 /* ignore because we don't parse all options */
 }
-- 
1.7.4.4

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