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

Re: [libvirt] [PATCH 04/10] python: Implement virStreamSend/Recv



On Mon, Jun 20, 2011 at 03:28:07PM -0400, Cole Robinson wrote:
> On 06/16/2011 10:44 AM, Daniel Veillard wrote:
> > On Wed, Jun 15, 2011 at 09:23:13PM -0400, Cole Robinson wrote:
> >> The return values for the python version are different that the C version
> >> of virStreamSend: on success we return a string, an error raises an exception,
> >> and if the stream would block we return int(-2). We need to do this
> >> since strings aren't passed by reference in python.
> > 
> >   I find this a bit bizarre, either we return a string or we return an
> > integer, but if we don't have any other way to provide the information
> > Since we provide the wrapper what about returning
> >   (code#, "string value")
> > allowing to have only one type on return instead (i.e. just change
> > recv() in the override below)
> > 
> 
> Certainly from a C perspective it's bizarre, but with dynamic typing I think
> this makes the interface much simpler without much confusion. -2 is just a
> special value here, much like "" means EOF. The code essentially looks like:
> 
> ret = stream.recv(1024)
> if ret == "":
>     return # EOL
> if ret == -2:
>     return # E_WOULDBLOCK
> 
> And only users of NONBLOCK flag need to handle this case. Even if they forget
> to do so, their code would likely error quickly if they try to perform any
> string operations on the returned value.
> 
> The benefit of returning a tuple is that it makes the multiple return values
> clear to an API user who doesn't read the docs, however I think it increases
> the chance of client bugs and makes client code uglier. Users who aren't using
> the NONBLOCK flag will still have to handle to at least acknowledge the return
> code which has no meaning in their case. For NONBLOCK users, what do we set
> the string value to if the error code is -2?
> 
> - string "": However this is a value with special meaning, and if the user
> mishandles the error code or outright doesn't check it, handling this value
> may cause hard do diagnose behavior
> - None: If the error code is mishandled, this would likely fail in the same
> way as the original proposal, which is okay. But if the user was simply doing
> a check like 'if not stringdata:' for an EOL check, they would incorrectly
> match this case as well, which has the same problem as "" (unlike a value of -2)

  Okay :-) ACK !

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]