Re: [libvirt] [PATCH libvirt-java] Return a byte[] array with secretGetValue

On 03/16/2012 04:08 AM, Wido den Hollander wrote: > > Signed-off-by: Wido den Hollander<wido(a)widodh.nl> > --- No commit message explaining why this is needed? > src/main/java/org/libvirt/Secret.java | 11 +++++++++-- > src/main/java/org/libvirt/jna/Libvirt.java | 2 +- > 2 files changed, 10 insertions(+), 3 deletions(-) Alas, it looks like we don't have an active libvirt-java maintainer participating on this list right now. Would you like to take over the role?

> > diff --git a/src/main/java/org/libvirt/Secret.java b/src/main/java/org/libvirt/Secret.java > index e536cf4..a874925 100644 > --- a/src/main/java/org/libvirt/Secret.java > +++ b/src/main/java/org/libvirt/Secret.java > @@ -5,6 +5,9 @@ import org.libvirt.jna.SecretPointer; > > 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; It's been a long time since I coded in Java - in fact, before java.nio was introduced. That said, > > /** > * A secret defined by libvirt > @@ -109,9 +112,13 @@ public class Secret { > * > * @return the value of the secret, or null on failure. > */ > - public String getValue() throws LibvirtException { > - String returnValue = libvirt.virSecretGetValue(VSP, new NativeLong(), 0); > + public byte[] getValue() throws LibvirtException { > + LongByReference value_size = new LongByReference(); > + Pointer value = libvirt.virSecretGetValue(VSP, value_size, 0); > processError(); > + ByteBuffer bb = value.getByteBuffer(0, value_size.getValue()); > + byte[] returnValue = new byte[bb.remaining()]; > + bb.get(returnValue); On the surface this looks reasonable. > return returnValue; > } > > diff --git a/src/main/java/org/libvirt/jna/Libvirt.java b/src/main/java/org/libvirt/jna/Libvirt.java > index 2c8c03d..3804d55 100644 > --- a/src/main/java/org/libvirt/jna/Libvirt.java > +++ b/src/main/java/org/libvirt/jna/Libvirt.java > @@ -330,7 +330,7 @@ public interface Libvirt extends Library { > public int virSecretGetUUID(SecretPointer virSecretPtr, byte[] uuidString); > public int virSecretGetUUIDString(SecretPointer virSecretPtr, byte[] uuidString); > public String virSecretGetUsageID(SecretPointer virSecretPtr); > - public String virSecretGetValue(SecretPointer virSecretPtr, NativeLong value_size, int flags); This is an API break. Deleting functions is generally bad for existing users, unless we have proven the old signature is completely unusable.

> + public Pointer virSecretGetValue(SecretPointer virSecretPtr, LongByReference value_size, int flags); And since your new function has a different signature, why not just keep both as an overloaded function?

> public String virSecretGetXMLDesc(SecretPointer virSecretPtr, int flags); > public SecretPointer virSecretLookupByUsage(ConnectionPointer virConnectPtr, int usageType, String usageID); > public SecretPointer virSecretLookupByUUID(ConnectionPointer virConnectPtr, byte[] uuidBytes);