Re: [PATCH 20/20] libxl: ao: Convert libxl_run_bootloader

["Roger Pau Monné" <roger.pau@entel.upc.edu>]

2012/3/22 Ian Jackson <Ian.Jackson@eu.citrix.com>:
> Roger Pau Monné writes ("Re: [Xen-devel] [PATCH 20/20] libxl: ao: Convert libxl_run_bootloader"):
>> > +    AC_CHECK_HEADER([libutil.h],[
>> > +      AC_DEFINE([INCLUDE_LIBUTIL_H],[<libutil.h>],[libutil header file name])
>> > +    ])
> ...
>> Shouldn't you add something like this (or perhaps add the necessary
>> AC_CHECK_HEADER for this headers, so we can get rid of the OS
>> conditional include dependency):
>>
>> #if defined(__NetBSD__) || defined(__OpenBSD__)
>> #include <util.h>
>> #elif defined(__linux__)
>> #include <pty.h>
>> #elif defined(__sun__)
>> #include <stropts.h>
>> #endif
>> #include <utmp.h>
>
> No, I don't think so.  Currently the only two platforms we really
> support are *BSD and Linux, and I think what I have (include
> <libutil.h> if it exists, and nothing non-portable otherwise) is
> correct.
>
> Does this test not work on *BSD ?

I haven't tried yet, in fact I have not tried this patch, it's just
that the manpage states that on NetBSD at least you have to include
util.h, it doesn't mention libutil.h.

http://netbsd.gw.com/cgi-bin/man-cgi?forkpty++NetBSD-current

And the linux man page states that pty.h and utmp.h
(http://linux.die.net/man/3/forkpty) should be included to use this
functions under Linux, but I think you have that covered. Anyway,
those includes are inside libxl_osdeps.h, so if they are not necessary
we should get rid of them, or add the necessary autoconf stuff to
handle that without all the OS conditionals.

> Certainly we don't want these kind of os-specific #ifdefs.  If there
> are multiple possible headers we should call AC_CHECK_HEADER for them.
>
> Ian.

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