Re: [xml] An xmlXPathTypeVal is never equal to XML_NAMESPACE_DECL

[Hans Wennborg <hans chromium org>]

On Wed, Sep 26, 2012 at 3:21 AM, Daniel Veillard <veillard redhat com> wrote:
> On Tue, Sep 25, 2012 at 04:21:34PM +0100, Hans Wennborg wrote:
> > Hi,
> >
> > We're using libxml2 in Chromium, and I just noticed that recent
> > versions of Clang have started warning about the following code:
> >
> > /usr/local/google/work/chrome/src/third_party/libxml/src/xpath.c:12269:13:
> > warning: comparison of constant 18 with expression of type
> > 'xmlXPathTypeVal' is always false
> > [-Wtautological-constant-out-of-range-compare]
> >                         if (type == XML_NAMESPACE_DECL)
> >                             ~~~~ ^  ~~~~~~~~~~~~~~~~~~
> >
> > In the latest version of xpath.c, the code is still there, but on line 12402.
> >
> >
> > Looking at the code, 'type' is a variable of type 'xmlXPathTypeVal',
> > whereas XML_NAMESPACE_DECL is an enumerator in 'xmlElementType'.
> >
> > Clang warns because the 'xmlXPathTypeVal' type is not wide enough to
> > hold the value of XML_NAMESPACE_DECL, and therefore the comparison
> > will never be true.
> >
> > I've tried to figure out what a good fix would be here, but I don't
> > know the code well enough. It would be great if someone could advice
> > on the best way to fix this.
>
>   You need a bit more context, the code fragment is:
>
>                     } else if (cur->type == type) {
>                         if (type == XML_NAMESPACE_DECL)
>                             XP_TEST_HIT_NS
>                         else
>                             XP_TEST_HIT
>                     } else if
>
>  So yes the comparison is done to the wrong type ... but the line
> before we checked that type and cur->type have the same value !
>
> We can change the test to:
>
>                         if (cur->type == XML_NAMESPACE_DECL)
>
> this will silent Clang, but the semantic should be exactly the same
> (as far as I know equality is transitive even in C :-)
> I'm  pushing the change,

Yup, that should fix it! Thanks very much.

 - Hans