warning on GCC 4.4.0 (win TDM)

Developer
Mihael
2009-10-30
2013-05-20
  • Mihael

    Mihael - 2009-10-30

    I get the following warning:

        tinyxmlparser.cpp|357|warning: suggest parentheses around '&&' within '||'|

    Which is easily fixed by adding parentheses around here:

        while ( *p && IsWhiteSpace( *p ) || *p == '\n' || *p =='\r' )

    Changing it from above to this:

        while ( (*p && IsWhiteSpace( *p )) || *p == '\n' || *p =='\r' )

    Its an easy fix, don't know why the warning should be there when it doesn't have to be.

     
  • Keith Marshall

    Keith Marshall - 2009-11-05

    This issue cropped up on the MinGW Developer's ML, only yesterday:
    http://thread.gmane.org/gmane.comp.gnu.mingw.devel/3520

    The warning occurs because the logical expression, as it stands, contains an ambiguous mix of AND and OR subexpressions, which the compiler cannot be certain to evaluate with the precedence the programmer intended, without parenthesis to specify this intent.

    Your parenthesis just affirm what the compiler will assume, but is that really what the programmer intended?  I think not; more likely is:

         while( *p && (IsWhiteSpace( *p ) || *p == '\n' || *p == '\r') )

    As it happens, in this case, the ORed subexpressions are redundant, so don't affect the outcome anyway, but the compiler has no way to know that; the entire expression may be safely simplified to:

         while( *p && IsWhiteSpace( *p ) )

    (ignoring the redundancy within IsWhiteSpace() itself - do you know of any implementation of isspace() which doesn't satisfy the POSIX requirement that '\n' and '\r' are included in the set of whitespace characters?)

     
  • Lee Thomason

    Lee Thomason - 2009-11-05

    Cleaning up the logic is a good thing - I agree the while() is poorly written.

    I remember the isspace() problem vaguely. The isspace() function is buggy and incorrectly returns 'false' for n/r on some platform. I forget which one, but that's bug fixes in an mature code base for you. :) TinyXML is run on some strange systems, and I try to accommodate that.

     
  • Lee Thomason

    Lee Thomason - 2010-03-14

    Okay - good catch. Once I sat down and looked at the code I saw what you meant. Fixed and checked in.

     

Log in to post a comment.