Re: [libvirt] [PATCH 4/4] qemu: fix a error coverd issue in 2 place

2015-02-26 Thread lhuang 


On 02/25/2015 10:51 PM, Ján Tomko wrote:

On Tue, Feb 24, 2015 at 01:52:08PM -0500, John Ferlan wrote:


On 02/13/2015 02:17 AM, Luyao Huang wrote:

we already set a more clearly error in networkGetNetworkAddress,
so this error will cover our error in networkGetNetworkAddress.

Signed-off-by: Luyao Huang lhu...@redhat.com
---
  src/qemu/qemu_command.c | 14 --
  1 file changed, 4 insertions(+), 10 deletions(-)


I didn't check every exit patch from networkGetNetworkAddress, but
perhaps these should change to :

 if (ret  0) {
 if (virGetLastError() == NULL)
 virReportError(VIR_ERR_XML_ERROR,...);
 goto error;
 }

Just to be sure we don't leave without setting some sort of error.

No.

All the exit paths should be checked to make sure an error was reported
in all cases where we return -1.


Okay, i have checked the code and think we will return -1 with an error 
setting.


So i will keep the code like this:
  if (ret  0)
  goto error;

Thanks for your review

Jan


Luyao

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

Re: [libvirt] [PATCH 4/4] qemu: fix a error coverd issue in 2 place

2015-02-25 Thread Ján Tomko 
On Tue, Feb 24, 2015 at 01:52:08PM -0500, John Ferlan wrote:
 
 
 On 02/13/2015 02:17 AM, Luyao Huang wrote:
  we already set a more clearly error in networkGetNetworkAddress,
  so this error will cover our error in networkGetNetworkAddress.
  
  Signed-off-by: Luyao Huang lhu...@redhat.com
  ---
   src/qemu/qemu_command.c | 14 --
   1 file changed, 4 insertions(+), 10 deletions(-)
  
 
 I didn't check every exit patch from networkGetNetworkAddress, but
 perhaps these should change to :
 
 if (ret  0) {
 if (virGetLastError() == NULL)
 virReportError(VIR_ERR_XML_ERROR,...);
 goto error;
 }
 
 Just to be sure we don't leave without setting some sort of error.

No.

All the exit paths should be checked to make sure an error was reported
in all cases where we return -1.

Jan


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 4/4] qemu: fix a error coverd issue in 2 place

2015-02-25 Thread lhuang 


On 02/25/2015 02:52 AM, John Ferlan wrote:


On 02/13/2015 02:17 AM, Luyao Huang wrote:

we already set a more clearly error in networkGetNetworkAddress,
so this error will cover our error in networkGetNetworkAddress.

Signed-off-by: Luyao Huang lhu...@redhat.com
---
  src/qemu/qemu_command.c | 14 --
  1 file changed, 4 insertions(+), 10 deletions(-)


I didn't check every exit patch from networkGetNetworkAddress, but
perhaps these should change to :

 if (ret  0) {
 if (virGetLastError() == NULL)
 virReportError(VIR_ERR_XML_ERROR,...);
 goto error;
 }

Just to be sure we don't leave without setting some sort of error.


Okay, thanks for your advise, i will change them in next version.


John

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6bd8f69..b42e0f4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7284,12 +7284,9 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr 
cfg,
 network driver not present));
  goto error;
  }
-if (ret  0) {
-virReportError(VIR_ERR_XML_ERROR,
-   _(listen network '%s' had no usable address),
-   listenNetwork);
+if (ret  0)
  goto error;
-}
+
  listenAddr = netAddr;
  /* store the address we found in the graphics element so it will
   * show up in status. */
@@ -7448,12 +7445,9 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr 
cfg,
 network driver not present));
  goto error;
  }
-if (ret  0) {
-virReportError(VIR_ERR_XML_ERROR,
-   _(listen network '%s' had no usable address),
-   listenNetwork);
+if (ret  0)
  goto error;
-}
+
  listenAddr = netAddr;
  /* store the address we found in the graphics element so it will
   * show up in status. */



Luyao

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

Re: [libvirt] [PATCH 4/4] qemu: fix a error coverd issue in 2 place

2015-02-24 Thread John Ferlan 


On 02/13/2015 02:17 AM, Luyao Huang wrote:
 we already set a more clearly error in networkGetNetworkAddress,
 so this error will cover our error in networkGetNetworkAddress.
 
 Signed-off-by: Luyao Huang lhu...@redhat.com
 ---
  src/qemu/qemu_command.c | 14 --
  1 file changed, 4 insertions(+), 10 deletions(-)
 

I didn't check every exit patch from networkGetNetworkAddress, but
perhaps these should change to :

if (ret  0) {
if (virGetLastError() == NULL)
virReportError(VIR_ERR_XML_ERROR,...);
goto error;
}

Just to be sure we don't leave without setting some sort of error.

John
 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 6bd8f69..b42e0f4 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -7284,12 +7284,9 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr 
 cfg,
 network driver not present));
  goto error;
  }
 -if (ret  0) {
 -virReportError(VIR_ERR_XML_ERROR,
 -   _(listen network '%s' had no usable 
 address),
 -   listenNetwork);
 +if (ret  0)
  goto error;
 -}
 +
  listenAddr = netAddr;
  /* store the address we found in the graphics element so it 
 will
   * show up in status. */
 @@ -7448,12 +7445,9 @@ 
 qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
 network driver not present));
  goto error;
  }
 -if (ret  0) {
 -virReportError(VIR_ERR_XML_ERROR,
 -   _(listen network '%s' had no usable address),
 -   listenNetwork);
 +if (ret  0)
  goto error;
 -}
 +
  listenAddr = netAddr;
  /* store the address we found in the graphics element so it will
   * show up in status. */
 

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