Re: [Xen-devel] [v5][PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Chen, Tiejun writes ("Re: [v5][PATCH 11/16] tools/libxl: detect and avoid 
conflicts with RDM"):
> On 2015/7/7 19:57, Ian Jackson wrote:
> > I think the error handling here is wrong.  Certainly `rc' should not
> > be used for a libxc return.
> 
> Nope, we don't return rc directly. In any case of error, rc is reset as 
> "ERROR_FAIL" finally.

Did you see

> > See tools/libxl/CODING_STYLE.  I know that much of the existing code
> > uses rc for libxc returns but this is deprecated, and please don't
> > make more of it.

this ?

> >> +    if (errno == ENOBUFS) {
> >> +        *xrdm = libxl__malloc(gc,
> >> +                              *nr_entries *
> >> +                              sizeof(xen_reserved_device_memory_t));
> >> +        rc = xc_reserved_device_memory_map(CTX->xch, flag, seg, bus, 
> >> devfn,
> >> +                                           *xrdm, nr_entries);
> >
> > This might be less code, and a bit clearer, if it were a loop rather
> > than two calls with the second in an if.
> 
> Sorry I can't understand completely what you want to do.

I meant that you could have a loop containing one call to
xc_reserved_device_memory_map.  The loop would normally execute twice.
But it was just a suggestion.  If you don't want to do it that way,
the existing arrangement is tolerable.

>      if (errno != ENOBUFS) {
>          rc = ERROR_FAIL;
>          goto out;
>      }

That would be clearer, yes.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel