Re: [Xen-devel] [PATCH 2/6] libxl: portiblity fixes

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Christoph Egger writes ("[Xen-devel] [PATCH 2/6] libxl: portiblity fixes"):
> --- a/tools/libxl/libxl_osdeps.h      Tue Jul 27 13:48:38 2010 +0200
> +++ b/tools/libxl/libxl_osdeps.h      Tue Jul 27 13:53:12 2010 +0200
> @@ -23,6 +23,8 @@
>  
>  #define _GNU_SOURCE
>  
> +#include <libxl_internal.h>
> +

This is wrong.  libxl_osdeps.h should not include libxl_internal.h.

> +/* libxl_blktap_enabled:
> + *    return true if blktap/blktap2 support is available.
> + */
> +int libxl_blktap_enabled(struct libxl_ctx *ctx);

This is not what libxl_osdeps.h is for.  These kind of functions can
be declared in a new section in libxl_internal.h.

Also, you should divide your patches conceptually, rather than
according to which files they touch.

This patch is wrong because it introduces a couple of function
declarations but it does not introduce the definitions; your later
patch which introduces the definitions is wrong because it introduces
some functions which are intended to replace existing code, but the
patch does not replace the existing code and the new functions are not
called anywhere in that patch.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel