[libvirt] [libvirt-java] [PATCH] Avoid calling processError for functions that cannot fail

The libvirt functions virNodeDeviceNumOfCaps, virNodeDeviceGetParent and virNodeDeviceListCaps never indicate an error because they do not fail. --- src/main/java/org/libvirt/Device.java | 19 ++++++------------- src/main/java/org/libvirt/jna/Libvirt.java | 2 +- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/libvirt/Device.java b/src/main/java/org/libvirt/Device.java index fe49ce9..5492b69 100644 --- a/src/main/java/org/libvirt/Device.java +++ b/src/main/java/org/libvirt/Device.java @@ -98,23 +98,19 @@ public class Device { /** * Returns the number of capabilities which the instance has. * - * @throws LibvirtException + * @throws LibvirtException <em>never</em> */ public int getNumberOfCapabilities() throws LibvirtException { - int num = libvirt.virNodeDeviceNumOfCaps(VDP); - processError(); - return num; + return libvirt.virNodeDeviceNumOfCaps(VDP); } /** * Returns the parent of the device * - * @throws LibvirtException + * @throws LibvirtException <em>never</em> */ public String getParent() throws LibvirtException { - String parent = libvirt.virNodeDeviceGetParent(VDP); - processError(); - return parent; + return libvirt.virNodeDeviceGetParent(VDP); } /** @@ -123,15 +119,13 @@ public class Device { * @throws LibvirtException */ public String getXMLDescription() throws LibvirtException { - String desc = libvirt.virNodeDeviceGetXMLDesc(VDP); - processError(); - return desc; + return processError(libvirt.virNodeDeviceGetXMLDesc(VDP)); } /** * List the capabilities of the device * - * @throws LibvirtException + * @throws LibvirtException <em>never</em> */ public String[] listCapabilities() throws LibvirtException { int maxCaps = getNumberOfCapabilities(); @@ -139,7 +133,6 @@ public class Device { if (maxCaps > 0) { libvirt.virNodeDeviceListCaps(VDP, names, maxCaps); - processError(); } return names; } diff --git a/src/main/java/org/libvirt/jna/Libvirt.java b/src/main/java/org/libvirt/jna/Libvirt.java index b026b04..a2bf42e 100644 --- a/src/main/java/org/libvirt/jna/Libvirt.java +++ b/src/main/java/org/libvirt/jna/Libvirt.java @@ -281,7 +281,7 @@ public interface Libvirt extends Library { String virNodeDeviceGetName(DevicePointer virDevicePointer); String virNodeDeviceGetParent(DevicePointer virDevicePointer); int virNodeDeviceNumOfCaps(DevicePointer virDevicePointer); - int virNodeDeviceListCaps(DevicePointer virDevicePointer, String[] names, int maxNames); + int virNodeDeviceListCaps(DevicePointer virDevicePointer, Pointer[] names, int maxNames); String virNodeDeviceGetXMLDesc(DevicePointer virDevicePointer); int virNodeDeviceFree(DevicePointer virDevicePointer); int virNodeDeviceDettach(DevicePointer virDevicePointer); -- 1.8.5.2.msysgit.0

At Wed, 8 Jan 2014 16:25:30 +0100, Claudio Bley wrote:
The libvirt functions virNodeDeviceNumOfCaps, virNodeDeviceGetParent and virNodeDeviceListCaps never indicate an error because they do not fail. --- src/main/java/org/libvirt/Device.java | 19 ++++++------------- src/main/java/org/libvirt/jna/Libvirt.java | 2 +- 2 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/src/main/java/org/libvirt/jna/Libvirt.java b/src/main/java/org/libvirt/jna/Libvirt.java index b026b04..a2bf42e 100644 --- a/src/main/java/org/libvirt/jna/Libvirt.java +++ b/src/main/java/org/libvirt/jna/Libvirt.java @@ -281,7 +281,7 @@ public interface Libvirt extends Library { String virNodeDeviceGetName(DevicePointer virDevicePointer); String virNodeDeviceGetParent(DevicePointer virDevicePointer); int virNodeDeviceNumOfCaps(DevicePointer virDevicePointer); - int virNodeDeviceListCaps(DevicePointer virDevicePointer, String[] names, int maxNames); + int virNodeDeviceListCaps(DevicePointer virDevicePointer, Pointer[] names, int maxNames); String virNodeDeviceGetXMLDesc(DevicePointer virDevicePointer); int virNodeDeviceFree(DevicePointer virDevicePointer); int virNodeDeviceDettach(DevicePointer virDevicePointer);
I just realized that this hunk does not belong to the patch. Please ignore it when reviewing. Claudio -- AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany Phone: +49 341 265 310 19 Web:<http://www.av-test.org> Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076) Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern

At Wed, 08 Jan 2014 16:31:38 +0100, Claudio Bley wrote:
At Wed, 8 Jan 2014 16:25:30 +0100, Claudio Bley wrote:
The libvirt functions virNodeDeviceNumOfCaps, virNodeDeviceGetParent and virNodeDeviceListCaps never indicate an error because they do not fail. --- src/main/java/org/libvirt/Device.java | 19 ++++++------------- src/main/java/org/libvirt/jna/Libvirt.java | 2 +- 2 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/src/main/java/org/libvirt/jna/Libvirt.java b/src/main/java/org/libvirt/jna/Libvirt.java index b026b04..a2bf42e 100644 --- a/src/main/java/org/libvirt/jna/Libvirt.java +++ b/src/main/java/org/libvirt/jna/Libvirt.java @@ -281,7 +281,7 @@ public interface Libvirt extends Library { String virNodeDeviceGetName(DevicePointer virDevicePointer); String virNodeDeviceGetParent(DevicePointer virDevicePointer); int virNodeDeviceNumOfCaps(DevicePointer virDevicePointer); - int virNodeDeviceListCaps(DevicePointer virDevicePointer, String[] names, int maxNames); + int virNodeDeviceListCaps(DevicePointer virDevicePointer, Pointer[] names, int maxNames); String virNodeDeviceGetXMLDesc(DevicePointer virDevicePointer); int virNodeDeviceFree(DevicePointer virDevicePointer); int virNodeDeviceDettach(DevicePointer virDevicePointer);
I just realized that this hunk does not belong to the patch. Please ignore it when reviewing.
Looking at this patch again made me think... Libvirt is a connection based (ie. client / server) system. Which means that basically *any* function could fail -- except those which don't require a valid connection. Self-NACK in this case. This is a bit misleading in the documentation; I would have assumed that a function documented as returning NULL representing "nothing found" has no other meaning of indicating an error. But, rather, it means either nothing found or an error occurred and I have to check for an error, right? Claudio -- AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany Phone: +49 341 265 310 19 Web:<http://www.av-test.org> Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076) Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern
participants (1)
-
Claudio Bley