Re: [PATCH 2/6] libxl: portiblity fixes

[Christoph Egger <Christoph.Egger@amd.com>]

On Tuesday 27 July 2010 18:34:21 Ian Jackson wrote:
> 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.

The function declarations are the API and the function defintions
are the OS dependent implementations of the API.
Implementations and use of the API is used in different patches.
This is my understanding of defining and implementing an API
in C.
blktap support for linux and netbsd are very different in their 
implementation.
In netbsd, blktap will be implemented using puffs
(http://netbsd.gw.com/cgi-bin/man-cgi?puffs+3+NetBSD-current)

Christoph

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632