On Fri, Jun 15, 2007 at 09:06:55AM +0100, Richard W.M. Jones wrote: > Daniel P. Berrange wrote: > >On Thu, Jun 14, 2007 at 05:26:39PM -0400, Mark Johnson wrote: > >>This patch has the includes need to build on Solaris. > >>I've been using ifdef linux & ifndef linux to distinguish > >>between solaris and linux at this point. > > > >Looks ok aside from > [..] > > No, I don't agree. We should use configure.in to test for the presence > of header files and then do things like: > > #ifdef HAVE_STRINGS_H > #include <strings.h> > #endif > > See the current configure.in, AC_CHECK_HEADERS. > > In fact we already have the HAVE_STRINGS_H symbol defined. Agreed. And in the case of system specific header use the system specific macro to guard e.g.: + +#ifndef __linux__ +#include <iso/limits_iso.h> +#endif should probably be rewritten as #ifdef __sun__ #include <iso/limits_iso.h> #endif or with whatever the macro should be on your platform, so that if someone tries to compile on say AIX it doesn't break on an inexistant header but rather if there is a missing reference in the code itself. thanks, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/