Re: [Xen-devel] [PATCH v4] libxc: check for various libxc failures and pass them down

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Sun, 2013-11-03 at 12:08 +1300, Matthew Daley wrote:
>  static int xc_cpuid_policy(
>      xc_interface *xch, domid_t domid,
>      const unsigned int *input, unsigned int *regs)
>  {
> +    int                 rc;
>      xc_dominfo_t        info;
>  
> -    if ( xc_domain_getinfo(xch, domid, 1, &info) == 0 )
> +    rc = xc_domain_getinfo(xch, domid, 1, &info);
> +    if ( rc < 0 )
> +        return rc;
> +    if ( rc == 0 )
>          return -EINVAL;

I think this means that this function now returns a mixture of -1 (with
errno set) and -errno depending on the failure mode. libxc really is a
horrible chuffing mess in this regard, sorry.

Luckily nothing right now looks at the return value (you are adding
those checks to the caller in this patch) so I think you can just
convert this into { errno=EINVAL; return -1; }

Then again, it looks like xc_cpuid_set (the aforementioned called)
already returns both styles (explicit rc = -EINVAL, vs returning the
result of do_domctl which is -1 and set errno). Gah!

Hah, but no one looks at the specific value of the result of
xc_cpuid_set on failure: libxl doesn't check it at all, Python and ocaml
bindings turn it into a generic exception using xc_get_last_error.

WTF, xc_get_last_error is a *third* error handling mechanism in libxc,
apparently built into the logging infrastructure. Which isn't used at
all on this code path AFAICT.

What a terrible mess. I think I should just apply this patch and putthe
rug back over the rest of it :-(

Ian.



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