Developer forums (C::B DEVELOPMENT STRICTLY!) > Development

GCC 4.3 Warnings

(1/3) > >>

rhf:
Hi guys,

I've been using CB for sometime now and feel a little guilty because I have not been contributing to your development work. Recently I did a fresh rebuild of Codeblocks with TDM's gcc4.3.2 and noticed quite a few warnings, such as "suggest parentheses around && within ||" and "suggest explicit braces to avoid ambiguous 'else'".

To help out, I thought I would prepare a patch to eliminate these warnings. However, most of these are in tinyxml, squirrel, or wxscintilla. Does it make sense to make changes in these files, or should such changes be directed to the developers of the programs?

Also, if there is a "real" potential ambiguity in these files, should this be brought to the attention of the CB team or to the original developer?

killerbot:
we are already fixing them, no at a fast pace. So any conribution might help ;-)
I know there's one in tinyxml, which is really a bug in tinyxml (which might be hard to occur though)

rhf:
OK. I submitted patch #2652 that fixes all but two of the warnings - maybe this will be helpful.
In a comment to the patch I mention the two warnings that I left alone because I was unsure of the intent of the code.

Jenna:
The first hunk of the patch (in tinyxmlparser.cpp) is most likely wrong, even if it manifests the way tinyxmlparser works for years.

I searched in the cvs-sources and found the revision where the (most likely) wrong behaviour came in.

Here are the original functions, before and after the change:

--- Code: ---http://tinyxml.cvs.sourceforge.net/viewvc/tinyxml/tinyxml/tinyxmlparser.cpp?revision=1.46&view=markup


  260 const char* TiXmlBase::SkipWhiteSpace( const char* p )
  261 {
  262 if ( !p || !*p )
  263 {
  264 return 0;
  265 }
  266 while ( p && *p )
  267 {
  268 // Skip the stupid Microsoft UTF-8 Byte order marks
  269 if ( *(p+0)==(char) 0xef
  270 && *(p+1)==(char) 0xbb
  271 && *(p+2)==(char) 0xbf )
  272 {
  273 p += 3;
  274 continue;
  275 }
  276 else if(*(p+0)==(char) 0xef
  277 && *(p+1)==(char) 0xbf
  278 && *(p+2)==(char) 0xbe )
  279 {
  280 p += 3;
  281 continue;
  282 }
  283 else if(*(p+0)==(char) 0xef
  284 && *(p+1)==(char) 0xbf
  285 && *(p+2)==(char) 0xbf )
  286 {
  287 p += 3;
  288 continue;
  289 }
  290
  291 if ( IsWhiteSpace( *p ) || *p == '\n' || *p =='\r' ) // Still using old rules for white space.
  292 ++p;
  293 else
  294 break;
  295 }
  296
  297 return p;
  298 }


http://tinyxml.cvs.sourceforge.net/viewvc/tinyxml/tinyxml/tinyxmlparser.cpp?revision=1.47&view=markup

 289 const char* TiXmlBase::SkipWhiteSpace( const char* p, TiXmlEncoding encoding )
  290 {
  291 if ( !p || !*p )
  292 {
  293 return 0;
  294 }
  295 if ( encoding == TIXML_ENCODING_UTF8 )
  296 {
  297 while ( *p )
  298 {
  299 // Skip the stupid Microsoft UTF-8 Byte order marks
  300 if ( *(p+0)==(char) 0xef
  301 && *(p+1)==(char) 0xbb
  302 && *(p+2)==(char) 0xbf )
  303 {
  304 p += 3;
  305 continue;
  306 }
  307 else if(*(p+0)==(char) 0xef
  308 && *(p+1)==(char) 0xbf
  309 && *(p+2)==(char) 0xbe )
  310 {
  311 p += 3;
  312 continue;
  313 }
  314 else if(*(p+0)==(char) 0xef
  315 && *(p+1)==(char) 0xbf
  316 && *(p+2)==(char) 0xbf )
  317 {
  318 p += 3;
  319 continue;
  320 }
  321
  322 if ( IsWhiteSpace( *p ) || *p == '\n' || *p =='\r' ) // Still using old rules for white space.
  323 ++p;
  324 else
  325 break;
  326 }
  327 }
  328 else
  329 {
  330 while ( *p && IsWhiteSpace( *p ) || *p == '\n' || *p =='\r' )
  331 ++p;
  332 }
  333
  334 return p;
  335 }

--- End code ---

I left them in fulllength, to make sure I did not oversee anything.
Please have a look.

thomas:

--- Quote from: rhf on January 18, 2009, 10:10:31 pm ---Also, if there is a "real" potential ambiguity in these files, should this be brought to the attention of the CB team or to the original developer?
--- End quote ---
No worries. Those warnings are somewhat silly, as there's no real ambiguity. The language standard is very clear about where an else belongs to, and it's also very clear about what something like while(foo = *bar) or if(a && b & c) means.
Nevertheless, silly as the warnings are, and redundant as the extra parentheses are in most cases, they may make code easier to understand for some people in some cases.

Navigation

[0] Message Index

[#] Next page