Re: RFC: Proposal to improve error reporting in libxl

[Ian Campbell <ian.campbell@citrix.com>]

On Wed, 2015-05-20 at 10:01 +0100, Euan Harris wrote:
> We would like to make libxl's error reporting more regular and informative
> for callers.

Yes, I agree this is a good idea.

>     We think we need to:
> 
>   * Make a list of the error conditions which can be encountered by
>     all the current API calls and define a set of error codes to
>     cover them.   Some codes will be generally applicable, but give
>     more information than a bare 'fail'; others may be specific to a
>     particular API call.   We may keep the existing FAIL, INVAL and 
>     NOMEM codes for times when they are appropriate.
> 
>     The error messages logged by each API call are a good starting point
>     for this list.   I have included a partial list below.
> 
>   * Change the API calls in point 1 above to use the new, more detailed
>     error codes.   These changes will break client code which checks for
>     particular error codes, rather than just checking whether the return
>     code is non-zero.

I think that at least for the case of turning a uselessly generic
ERROR_FAIL into something specific this is tolerable.

I'm not sure if we will want a raft of LIBXL_HAVE_ERROR_* defines. It
seems not all that useful.

(Perhaps every enum value FOO in the IDL should get a #define FOO FOO to
make this automatic?)

>   * For the API calls in points 2 and 3 which can encounter errors
>     which callers might care about, change their interfaces to return
>     error codes.   For functions which previously returned pointers,
>     add pointer-to-pointer parameters.

I think this is doable using the LIBXL_API_VERSION arrangements
(described in libxl.h)

Essential for an existing
        THING *get_a_thing_list(ctx, int *nr_r)
which you are converting to
        int get_a_thing_list(ctx, THING **list_r, int *nr_r)

you would do something like:

#if LIBXL_API_VERSION < 0x406000 /* or whatever version */
static inline THING *get_a_thing_list(ctx, int *nr_r)
{
        THING *list = NULL
        int rc = get_a_thing_list(ctx, &list, nr_r);
        if ( rc )
                return NULL
        else
                return list;
}
#endif

Or something along those lines.

>   * For error codes returned by utility functions, described in point 4, 
>     decide whether the code can be reported directly or an API-level
>     error should be reported instead.

I think this fits in with the cleanups made in the second bullet and is
likewise fine. (But worth considering separately I agree)

Ian.