Re: Protect included headers with the proper autoconf macros

[Jacek Caban]

Hello.

Vincent Béron wrote:
> Changelog:
> Protect headers with proper autoconf macros.
>
> Vincent
> Index: dlls/msxml3/msxml_private.h
>
> RCS file: /home/wine/wine/dlls/msxml3/msxml_private.h,v
> retrieving revision 1.5
> diff -u -p -r1.5 msxml_private.h
> --- dlls/msxml3/msxml_private.h	9 Aug 2005 11:17:47 -0000	1.5
> +++ dlls/msxml3/msxml_private.h	9 Aug 2005 21:56:24 -0000
> @@ -23,9 +23,11 @@
>
> #include "xmldom.h"
>
> -#ifdef HAVE_LIBXML2
> -
>
> +#ifdef HAVE_LIBXML_PARSER_H
> #include <libxml/parser.h>
> +#endif
> -
>
> +#ifdef HAVE_LIBXML2
>
> extern HRESULT DOMElement_create( IXMLDOMElement** DOMElement, xmlDocPtr xmldoc );
> extern HRESULT NodeMap_create(IXMLDOMNamedNodeMap** DomNamedNodeMap, xmlDocPtr xmldoc, xmlNodePtr node );

This patch causes the compile error for me. I have libxml2 headers
but I don't have a library. XML2INCL is defined only if the configure
script found library so gcc cannot find my headers at compile time.
If you want to change it, you'll have to change configure.ac as well,
but why do you want to include libxml if it won't be used anyway?

Jacek

Protect included headers with the proper autoconf macros

[Mike McCormack]

> > -#ifdef HAVE_LIBXML2
> > -
> >
> > +#ifdef HAVE_LIBXML_PARSER_H
> > #include <libxml/parser.h>
> > +#endif
> > -
> >
> > +#ifdef HAVE_LIBXML2
> This patch causes the compile error for me. I have libxml2 headers
> but I don't have a library. XML2INCL is defined only if the configure
> script found library so gcc cannot find my headers at compile time.
> If you want to change it, you'll have to change configure.ac as well,
> but why do you want to include libxml if it won't be used anyway?

Probably better to do:

#ifdef HAVE_LIBXML2

#ifdef HAVE_LIBXML_PARSER_H
#include <libxml/parser.h>
#endif

...

#endif

That way we have the right include guard and we'll only compile if we 
have the library too.

Mike

Protect included headers with the proper autoconf macros

[Jacek Caban]

Mike McCormack wrote:
> > This patch causes the compile error for me. I have libxml2 headers
> > but I don't have a library. XML2INCL is defined only if the configure
> > script found library so gcc cannot find my headers at compile time.
> > If you want to change it, you'll have to change configure.ac as well,
> > but why do you want to include libxml if it won't be used anyway?
>
> Probably better to do:
>
> #ifdef HAVE_LIBXML2
>
> #ifdef HAVE_LIBXML_PARSER_H
> #include <libxml/parser.h>
> #endif
>
> ...
>
> #endif
>
> That way we have the right include guard and we'll only compile if we 
> have the library too.
>
> Mike

But it won't change anything as if HAVE_LIBXML2 is defined
then HAVE_LIBXML_PARSER_H is defined too, so
#ifdef HAVE_LIBXML_PARSER_H will be always true here.

Jacek

Protect included headers with the proper autoconf macros

[Francois Gouget]

On Wed, 10 Aug 2005, Jacek Caban wrote:
[...]
> But it won't change anything as if HAVE_LIBXML2 is defined
> then HAVE_LIBXML_PARSER_H is defined too, so
> #ifdef HAVE_LIBXML_PARSER_H will be always true here.

Yes but if you remove the #ifdef HAVE_LIBXML_PARSER_H check 
winapi_check will rightly complain that the include is unprotected 
because it, nor the casual reader, will know all the complex relations 
between HAVE_LIBXML_PARSER_H and HAVE_LIBXML2.

Protect included headers with the proper autoconf macros

[Jacek Caban]

Francois Gouget wrote:
> On Wed, 10 Aug 2005, Jacek Caban wrote:
> [...]
> > But it won't change anything as if HAVE_LIBXML2 is defined
> > then HAVE_LIBXML_PARSER_H is defined too, so
> > #ifdef HAVE_LIBXML_PARSER_H will be always true here.
>
> Yes but if you remove the #ifdef HAVE_LIBXML_PARSER_H check 
> winapi_check will rightly complain that the include is unprotected 
> because it, nor the casual reader, will know all the complex relations 
> between HAVE_LIBXML_PARSER_H and HAVE_LIBXML2.

Ah, I see. So Mike's solution is good for this.

Jacek