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

Re: [libvirt] [PATCH] Initial work on the JNA port, still a work in progress



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/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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