On Mon, Jul 27, 2009 at 07:05:05PM -0400, Bryan Kearney wrote:
> Daniel Veillard wrote:
>>   I think there are some cleanups needed w.r.t. spaces at end of lines 
>
>
> All whitespace errors were removed.

  thanks !

[...]
>>> -   private long VNP;
>>> +   protected Pointer VNP;
>>
>>   For my own education, that means that subclasses implementations may
>>   still use iit instead of keeping it fully private, right ?
>
>
> Correct. Makes this much more friendly for subclassing.
>

  Okay :-)

>>
>>> @@ -18,9 +28,10 @@ public class Network {
>>>      * @param virConnect
>>>      * @param VNP
>>>      */
>>> -   Network(Connect virConnect, long VNP){
>>> +   Network(Connect virConnect, Pointer VNP){       
>>>             this.virConnect = virConnect;
>>>             this.VNP = VNP;
>>> +           this.libvirt = virConnect.libvirt ;
>>>     }
>>
>>   I think we are slightly breaking the API here but in a way that should
>>   be compatible with existing code, since VNP was returned from the
>>   library, right ?
>
>
> We are. The core libvirt classes now return Pointers. So, the only issue  
> is if the caller was storing the pointer in their own code. Hopefully,  
> this was not being done.

  yeah I would guess that breakage is within acceptable range.

>>> @@ -34,6 +36,20 @@ public class NodeInfo {
>>>      */
>>>     public int threads;
>>>  +  
>>> +   public NodeInfo() {
>>> +   }
>>> +   
>>> +   public NodeInfo(virNodeInfo vInfo) {
>>> +//     this.model = new String(vInfo.model) ;
>>
>>   err, why ?
>>
>
> This is now a call to the JNA Native.toString() call. It abstracts out  
> some of the nuttiness of crossing the UTF-8 boundary.

  Okay

>>   I just hope that at the end of the patch series the test file is back
>>   to its original state. Maybe not worth fixing in all intermediary
>>   steps ...
>>
>
> Yes, the last patch set has the entire test class un-commented.
>

  Excellent :-)

   thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

Reply via email to