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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxx>
---
  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();


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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]