[libvirt] [PATCH] Fix checking of return codes in dispatcher

[Daniel P. Berrange]

On Wed, Apr 13, 2011 at 05:29:16PM -0600, Eric Blake wrote:
> On 04/13/2011 12:13 PM, Daniel P. Berrange wrote:
> > The libvirt APIs reserve any negative value for indicating an
> > error. Thus checks
> > 
> >     if (virXXXX() == -1)
> > 
> > Should instead be
> > 
> >     if (virXXXX() < 0)
> > 
> > * daemon/remote.c: s/ == -1/ < 0/
> > ---
> >  daemon/remote.c |  339 +++++++++++++++++++++++++++++--------------------------
> >  1 files changed, 180 insertions(+), 159 deletions(-)
> 
> Why the change in lines?
> 
> > @@ -3661,6 +3662,7 @@ remoteDispatchListDefinedNetworks(struct qemud_server *server ATTRIBUTE_UNUSED,
> >                                    remote_list_defined_networks_ret *ret)
> >  {
> >      int rv = -1;
> > +    int len;
> >  
> >      if (!conn) {
> >          virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
> > @@ -3679,12 +3681,12 @@ remoteDispatchListDefinedNetworks(struct qemud_server *server ATTRIBUTE_UNUSED,
> >          goto cleanup;
> >      }
> >  
> > -    ret->names.names_len =
> > -        virConnectListDefinedNetworks(conn,
> > +    len = virConnectListDefinedNetworks(conn,
> >                                        ret->names.names_val, args->maxnames);
> > -    if (ret->names.names_len == -1) {
> > +    if (len < 0) {
> >          goto cleanup;
> >      }
> > +    ret->names.names_len = len;
> 
> Oh, I see.  You didn't want to change ret until you know the value is valid.

It is a little worse than that.  'ret->names.names_len'
is actually an 'unsigned int', and the old code was
comparing that to '-1'. It worked, but it is kinda nasty
to rely on signed/unsigned casts in this way. So I decided
it was nicer to check for "< -1" with a signed variable,
before assigning to the unsigned variable.

> ACK.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|