On 05-07-12 10:20, Daniel Veillard wrote:

On Tue, Jun 26, 2012 at 10:07:31AM +0200, Wido den Hollander wrote:

...

We break the API with this, but Java does not support multiple method signatures with different return types.

The old method returned a String, but since a secret can be binary data this type is not suited.

Users who now that their secret is in fact a String, can use cast with:

Secret secret = conn.secretLookupByUUIDString("uuuuuuuid"); String value = new String(secret.getValue());

While it makes perfect sense to switch the jna layer to byte array as otherwise we can't guarantee functional operation in all cases, I aslo think we should not break compatibility, since Java doesn't allow overload with different value type, it just mean we need a new method name.

Yes, I get that. We don't want to break anything here.

So I'm simply renaming your method name to "getByteValue" but keeping getValue() implemented with the cast you suggest, that seems to work for me

I tested it and verified it's working for me with cephx authentication keys. Wido

...

Signed-off-by: Wido den Hollander <wido@widodh.nl> --- src/main/java/org/libvirt/Secret.java | 13 ++++++++++--- src/main/java/org/libvirt/jna/Libvirt.java | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/main/java/org/libvirt/Secret.java b/src/main/java/org/libvirt/Secret.java index 48f7895..39d9122 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;

/** * A secret defined by libvirt @@ -106,12 +109,16 @@ public class Secret {

/** * Fetches the value of the 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); return returnValue; }

diff --git a/src/main/java/org/libvirt/jna/Libvirt.java b/src/main/java/org/libvirt/jna/Libvirt.java index b1e53a2..f53199d 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); + public Pointer virSecretGetValue(SecretPointer virSecretPtr, LongByReference value_size, int flags); public String virSecretGetXMLDesc(SecretPointer virSecretPtr, int flags); public SecretPointer virSecretLookupByUsage(ConnectionPointer virConnectPtr, int usageType, String usageID); public SecretPointer virSecretLookupByUUID(ConnectionPointer virConnectPtr, byte[] uuidBytes); -- 1.7.9.5

So I'm squashing the following on top of your patch. I will reword the commit message slightly too. I'm sorry that this mean you will have to rename getValue() to getByteValue() but I prefer that than risking to break an unknown amount of existing users ... I hope you understand.

diff --git a/src/main/java/org/libvirt/Secret.java b/src/main/java/org/libvirt/Secret.java index 39d9122..561c258 100644 --- a/src/main/java/org/libvirt/Secret.java +++ b/src/main/java/org/libvirt/Secret.java @@ -108,11 +108,23 @@ public class Secret { }

/** - * Fetches the value of the secret + * Fetches the value of the secret as a string (note that + * this may not always work and getByteValue() is more reliable) + * This is just kept for backward compatibility * * @return the value of the secret, or null on failure. */ - public byte[] getValue() throws LibvirtException { + public String getValue() throws LibvirtException { + String returnValue = new String(getByteValue()); + return returnValue; + } + + /** + * Fetches the value of the secret as a byte array + * + * @return the value of the secret, or null on failure. + */ + public byte[] getByteValue() throws LibvirtException { LongByReference value_size = new LongByReference(); Pointer value = libvirt.virSecretGetValue(VSP, value_size, 0); processError();