[libvirt] [libvirt-java] [PATCH] Fix wrapping of native size_t data type

Libvirt function parameters having type (pointer to) size_t were wrapped via JNA using int, long or even NativeLong. Alas, none of these is actually correct as the size of size_t may be the same as the size of either (unsigned) int, long or even long long on different platforms. JNA provides the size of a native size_t to us, so using this information we define and use class SizeT and class SizeTByReference for wrapping a native size_t and size_t*, respectively. Signed-off-by: Claudio Bley <cbley@av-test.de> --- src/main/java/org/libvirt/Domain.java | 5 ++-- src/main/java/org/libvirt/Secret.java | 10 ++++---- src/main/java/org/libvirt/Stream.java | 7 +++-- src/main/java/org/libvirt/jna/Libvirt.java | 14 +++++----- src/main/java/org/libvirt/jna/SizeT.java | 25 ++++++++++++++++++ .../java/org/libvirt/jna/SizeTByReference.java | 27 ++++++++++++++++++++ 6 files changed, 70 insertions(+), 18 deletions(-) create mode 100644 src/main/java/org/libvirt/jna/SizeT.java create mode 100644 src/main/java/org/libvirt/jna/SizeTByReference.java diff --git a/src/main/java/org/libvirt/Domain.java b/src/main/java/org/libvirt/Domain.java index a72e8c6..efac9b9 100644 --- a/src/main/java/org/libvirt/Domain.java +++ b/src/main/java/org/libvirt/Domain.java @@ -3,6 +3,7 @@ package org.libvirt; import org.libvirt.jna.DomainPointer; import org.libvirt.jna.DomainSnapshotPointer; import org.libvirt.jna.Libvirt; +import org.libvirt.jna.SizeT; import org.libvirt.jna.virDomainBlockInfo; import org.libvirt.jna.virDomainBlockStats; import org.libvirt.jna.virDomainInfo; @@ -234,7 +235,7 @@ public class Domain { */ public DomainBlockStats blockStats(String path) throws LibvirtException { virDomainBlockStats stats = new virDomainBlockStats(); - int success = libvirt.virDomainBlockStats(VDP, path, stats, stats.size()); + int success = libvirt.virDomainBlockStats(VDP, path, stats, new SizeT(stats.size())); processError(); return success == 0 ? new DomainBlockStats(stats) : null; } @@ -695,7 +696,7 @@ public class Domain { */ public DomainInterfaceStats interfaceStats(String path) throws LibvirtException { virDomainInterfaceStats stats = new virDomainInterfaceStats(); - libvirt.virDomainInterfaceStats(VDP, path, stats, stats.size()); + libvirt.virDomainInterfaceStats(VDP, path, stats, new SizeT(stats.size())); processError(); return new DomainInterfaceStats(stats); } diff --git a/src/main/java/org/libvirt/Secret.java b/src/main/java/org/libvirt/Secret.java index 5332e02..4edb515 100644 --- a/src/main/java/org/libvirt/Secret.java +++ b/src/main/java/org/libvirt/Secret.java @@ -2,11 +2,11 @@ package org.libvirt; import org.libvirt.jna.Libvirt; import org.libvirt.jna.SecretPointer; +import org.libvirt.jna.SizeT; +import org.libvirt.jna.SizeTByReference; import static org.libvirt.Library.libvirt; import com.sun.jna.Native; -import com.sun.jna.NativeLong; -import com.sun.jna.ptr.LongByReference; import com.sun.jna.Pointer; import java.nio.ByteBuffer; @@ -120,7 +120,7 @@ public class Secret { * @return the value of the secret, or null on failure. */ public byte[] getByteValue() throws LibvirtException { - LongByReference value_size = new LongByReference(); + SizeTByReference value_size = new SizeTByReference(); Pointer value = libvirt.virSecretGetValue(VSP, value_size, 0); processError(); ByteBuffer bb = value.getByteBuffer(0, value_size.getValue()); @@ -154,7 +154,7 @@ public class Secret { * @return 0 on success, -1 on failure. */ public int setValue(String value) throws LibvirtException { - int returnValue = libvirt.virSecretSetValue(VSP, value, new NativeLong(value.length()), 0); + int returnValue = libvirt.virSecretSetValue(VSP, value, new SizeT(value.length()), 0); processError(); return returnValue; } @@ -165,7 +165,7 @@ public class Secret { * @return 0 on success, -1 on failure. */ public int setValue(byte[] value) throws LibvirtException { - int returnValue = libvirt.virSecretSetValue(VSP, value, new NativeLong(value.length), 0); + int returnValue = libvirt.virSecretSetValue(VSP, value, new SizeT(value.length), 0); processError(); return returnValue; } diff --git a/src/main/java/org/libvirt/Stream.java b/src/main/java/org/libvirt/Stream.java index 84e300c..d852d0d 100644 --- a/src/main/java/org/libvirt/Stream.java +++ b/src/main/java/org/libvirt/Stream.java @@ -1,11 +1,10 @@ package org.libvirt; import org.libvirt.jna.Libvirt; +import org.libvirt.jna.SizeT; import org.libvirt.jna.StreamPointer; import static org.libvirt.Library.libvirt; -import com.sun.jna.NativeLong; - public class Stream { public static int VIR_STREAM_NONBLOCK = (1 << 0); @@ -108,7 +107,7 @@ public class Stream { * @throws LibvirtException */ public int receive(byte[] data) throws LibvirtException { - int returnValue = libvirt.virStreamRecv(VSP, data, new NativeLong(data.length)); + int returnValue = libvirt.virStreamRecv(VSP, data, new SizeT(data.length)); processError(); return returnValue; } @@ -151,7 +150,7 @@ public class Stream { * @throws LibvirtException */ public int send(String data) throws LibvirtException { - int returnValue = libvirt.virStreamSend(VSP, data, new NativeLong(data.length())); + int returnValue = libvirt.virStreamSend(VSP, data, new SizeT(data.length())); processError(); return returnValue; } diff --git a/src/main/java/org/libvirt/jna/Libvirt.java b/src/main/java/org/libvirt/jna/Libvirt.java index 8c7d6ef..34ef966 100644 --- a/src/main/java/org/libvirt/jna/Libvirt.java +++ b/src/main/java/org/libvirt/jna/Libvirt.java @@ -180,7 +180,7 @@ public interface Libvirt extends Library { int virDomainAbortJob(DomainPointer virDomainPtr); int virDomainAttachDevice(DomainPointer virDomainPtr, String deviceXML); int virDomainAttachDeviceFlags(DomainPointer virDomainPtr, String deviceXML, int flags); - int virDomainBlockStats(DomainPointer virDomainPtr, String path, virDomainBlockStats stats, int size); + int virDomainBlockStats(DomainPointer virDomainPtr, String path, virDomainBlockStats stats, SizeT size); int virDomainBlockResize(DomainPointer virDomainPtr, String disk, long size, int flags); int virDomainCoreDump(DomainPointer virDomainPtr, String to, int flags); int virDomainCreate(DomainPointer virDomainPtr); @@ -210,7 +210,7 @@ public interface Libvirt extends Library { Pointer virDomainGetXMLDesc(DomainPointer virDomainPtr, int flags); int virDomainHasCurrentSnapshot(DomainPointer virDomainPtr, int flags); int virDomainHasManagedSaveImage(DomainPointer virDomainPtr, int flags); - int virDomainInterfaceStats(DomainPointer virDomainPtr, String path, virDomainInterfaceStats stats, int size); + int virDomainInterfaceStats(DomainPointer virDomainPtr, String path, virDomainInterfaceStats stats, SizeT size); int virDomainIsActive(DomainPointer virDomainPtr); int virDomainIsPersistent(DomainPointer virDomainPtr); DomainPointer virDomainLookupByID(ConnectionPointer virConnectPtr, int id); @@ -351,13 +351,13 @@ public interface Libvirt extends Library { int virSecretGetUUID(SecretPointer virSecretPtr, byte[] uuidString); int virSecretGetUUIDString(SecretPointer virSecretPtr, byte[] uuidString); String virSecretGetUsageID(SecretPointer virSecretPtr); - Pointer virSecretGetValue(SecretPointer virSecretPtr, LongByReference value_size, int flags); + Pointer virSecretGetValue(SecretPointer virSecretPtr, SizeTByReference value_size, int flags); String virSecretGetXMLDesc(SecretPointer virSecretPtr, int flags); SecretPointer virSecretLookupByUsage(ConnectionPointer virConnectPtr, int usageType, String usageID); SecretPointer virSecretLookupByUUID(ConnectionPointer virConnectPtr, byte[] uuidBytes); SecretPointer virSecretLookupByUUIDString(ConnectionPointer virConnectPtr, String uuidstr); - int virSecretSetValue(SecretPointer virSecretPtr, String value, NativeLong value_size, int flags); - int virSecretSetValue(SecretPointer virSecretPtr, byte[] value, NativeLong value_size, int flags); + int virSecretSetValue(SecretPointer virSecretPtr, String value, SizeT value_size, int flags); + int virSecretSetValue(SecretPointer virSecretPtr, byte[] value, SizeT value_size, int flags); int virSecretUndefine(SecretPointer virSecretPtr); //Stream Methods @@ -369,9 +369,9 @@ public interface Libvirt extends Library { int virStreamFinish(StreamPointer virStreamPtr) ; int virStreamFree(StreamPointer virStreamPtr) ; StreamPointer virStreamNew(ConnectionPointer virConnectPtr, int flags) ; - int virStreamSend(StreamPointer virStreamPtr, String data, NativeLong size); + int virStreamSend(StreamPointer virStreamPtr, String data, SizeT size); int virStreamSendAll(StreamPointer virStreamPtr, Libvirt.VirStreamSourceFunc handler, Pointer opaque); - int virStreamRecv(StreamPointer virStreamPtr, byte[] data, NativeLong length); + int virStreamRecv(StreamPointer virStreamPtr, byte[] data, SizeT length); int virStreamRecvAll(StreamPointer virStreamPtr, Libvirt.VirStreamSinkFunc handler, Pointer opaque); //DomainSnapshot Methods diff --git a/src/main/java/org/libvirt/jna/SizeT.java b/src/main/java/org/libvirt/jna/SizeT.java new file mode 100644 index 0000000..298fe73 --- /dev/null +++ b/src/main/java/org/libvirt/jna/SizeT.java @@ -0,0 +1,25 @@ +package org.libvirt.jna; + +import com.sun.jna.Native; +import com.sun.jna.IntegerType; + +/** + * Represents the native {@code size_t} data type. + */ +public final class SizeT extends IntegerType { + public SizeT() { this(0); } + public SizeT(long value) { + /* N.B. A three argument constructor is supported starting with + * JNA version 3.4.1. + * + * The third argument determines whether this class represents + * an unsigned integer type. When extracting a value into a + * larger-sized container (e.g. 4 byte native type into Java + * long), the value is properly converted as unsigned. + * + * TODO: Use this constructor once we require JNA >= 3.4.1 + */ + // super(Native.SIZE_T_SIZE, value, true); + super(Native.SIZE_T_SIZE, value); + } +} diff --git a/src/main/java/org/libvirt/jna/SizeTByReference.java b/src/main/java/org/libvirt/jna/SizeTByReference.java new file mode 100644 index 0000000..63fe453 --- /dev/null +++ b/src/main/java/org/libvirt/jna/SizeTByReference.java @@ -0,0 +1,27 @@ +package org.libvirt.jna; + +import com.sun.jna.Native; +import com.sun.jna.ptr.ByReference; + +/** + * Represents a native (call-by-reference) pointer to {@code size_t} data type. + */ +public final class SizeTByReference extends ByReference { + public SizeTByReference() { + this(0); + } + + public SizeTByReference(long value) { + super(Native.SIZE_T_SIZE); + setValue(value); + } + + public void setValue(long value) { + getPointer().setLong(0, value); + } + + public long getValue() { + return getPointer().getLong(0); + } + +} -- 1.7.9.5

At Wed, 24 Apr 2013 15:43:42 +0200, Claudio Bley wrote:
Libvirt function parameters having type (pointer to) size_t were wrapped via JNA using int, long or even NativeLong. Alas, none of these is actually correct as the size of size_t may be the same as the size of either (unsigned) int, long or even long long on different platforms.
JNA provides the size of a native size_t to us, so using this information we define and use class SizeT and class SizeTByReference for wrapping a native size_t and size_t*, respectively.
Ping? 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

On 04/24/2013 07:43 AM, Claudio Bley wrote:
Libvirt function parameters having type (pointer to) size_t were wrapped via JNA using int, long or even NativeLong. Alas, none of these is actually correct as the size of size_t may be the same as the size of either (unsigned) int, long or even long long on different platforms.
JNA provides the size of a native size_t to us, so using this information we define and use class SizeT and class SizeTByReference for wrapping a native size_t and size_t*, respectively.
Signed-off-by: Claudio Bley <cbley@av-test.de> ---
+public final class SizeT extends IntegerType { + public SizeT() { this(0); } + public SizeT(long value) { + /* N.B. A three argument constructor is supported starting with + * JNA version 3.4.1. + * + * The third argument determines whether this class represents + * an unsigned integer type. When extracting a value into a + * larger-sized container (e.g. 4 byte native type into Java + * long), the value is properly converted as unsigned. + * + * TODO: Use this constructor once we require JNA >= 3.4.1 + */ + // super(Native.SIZE_T_SIZE, value, true); + super(Native.SIZE_T_SIZE, value);
Can't you enforce proper expansion yourself, something like: super(Native.SIZE_T_SIZE, Native.SIZE_T_SIZE == 4 ? value & 0xffffffffL : value); That way, even on a 32-bit platform, if a caller passes in a size_t value of 0x80000000, but it gets sign-extended in conversion to Java long, you undo the sign extension and leave things with a positive value even without relying on the 3-argument superconstructor. Then again, it probably won't ever matter - if you have a 32-bit size_t, you are probably on a 32-bit platform where it is impossible to address that much memory in one object anyways, so it is unlikely that bug-free code will ever be passing a size_t value that large. If we argue that a large value is already buggy, and that we would eventually reject it as an error anyways, then it shouldn't matter if it accidentally gets sign extended into an even larger invalid value. Other than that, this patch looks okay to me. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Claudio Bley
-
Eric Blake