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:
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 }
I left them in fulllength, to make sure I did not oversee anything.
Please have a look.
well, if you look at the code above in tinyxmlparser.cpp
while ( *p )
{
// blablablablablablablabla
if ( IsWhiteSpace( *p ) || *p == '\n' || *p =='\r' ) // Still using old rules for white space.
++p;
else
break;
}
==> so first *p and then if(.....) ===> *p && (....)
you will see that this is not what was intended :
while ( *p && IsWhiteSpace( *p ) || *p == '\n' || *p =='\r' )
++p;
and it should be :
while ( *p && (IsWhiteSpace( *p ) || *p == '\n' || *p =='\r') )
++p;
only continue the rest of the test if *p is not 0;
This bug will hardly occur or have severe damage (if any0, but it is for sure wrong. And therefor a very good thing that GCC warns about this.
EDIT : I just looked at the patch part from "rhf", and that solution is not correct in my opinion.
while ( *p && IsWhiteSpace( *p ) || *p == '\n' || *p =='\r' )
++p;
Actually it doesn't make any difference whether or where you put additional parentheses in there, because *p == '\n' of course includes *p != '\0' and also IsWhiteSpace() (which relies on isspace) includes *p != '\0' as well, moreover explicitely includes '\r\n' too, which means the check for '\r\n' is twice redundant because isspace includes '\r\n' already tpp.
As bottom line, the above code is logically identical to just
while (IsWhiteSpace(*p))
++p;