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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list