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

[Wido den Hollander]

Hi,

On 05/26/2012 04:28 AM, Eric Blake wrote:
> On 03/16/2012 04:08 AM, Wido den Hollander wrote:
>>
>> Signed-off-by: Wido den Hollander<wido at 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?
>

I would like to take over that role, but I'm quit busy with some other 
projects like Ceph and CloudStack. However, CloudStack uses the 
libvirt-java bindings very heavily, so with that in mind it would be a 
good oppertunity.

The bindings seem quit old indeed and need updating.

I would like to become the maintainer, but I can't promise I'll have 
them updated within a short matter of time.

>>
>> 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.

You are right, I shouldn't have broken the API here.

>
>> +    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?

That is what I should have done indeed.

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