Issue 96059 - transex3: dodgy use of && and ||
Summary: transex3: dodgy use of && and ||
Status: CLOSED FIXED
Alias: None
Product: Internationalization
Classification: Code
Component: code (show other issues)
Version: DEV300m35
Hardware: All Linux, all
: P3 Trivial (vote)
Target Milestone: ---
Assignee: dtardon
QA Contact: issues@l10n
URL:
Keywords:
Depends on:
Blocks: 96084
  Show dependency tree
 
Reported: 2008-11-10 17:03 UTC by caolanm
Modified: 2013-08-07 15:02 UTC (History)
1 user (show)

See Also:
Issue Type: PATCH
Latest Confirmation in: ---
Developer Difficulty: ---

Attachments
what we probably want to do (1.04 KB, patch)
2008-11-10 17:04 UTC, caolanm 		no flags Details | Diff
fix of the fix :) (1.14 KB, patch)
2008-11-11 06:50 UTC, dtardon 		no flags Details | Diff
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this issue.
Description caolanm 2008-11-10 17:03:21 UTC 
in transex3/source/export2.cxx we have...

if( i+2 < rString.Len() &&
   rString.GetChar( i+1 ) == 'b' || rString.GetChar( i+1 ) == 'B' &&
   rString.GetChar( +2 ) == '>' )

i.e. 

if (a && b || c && d)

rules of precedence means this is equivalent to 

if ( (a && b) || (c && d) )

i.e. 

if( (i+2 < rString.Len() &&
   rString.GetChar( i+1 ) == 'b') || (rString.GetChar( i+1 ) == 'B' &&
   rString.GetChar( +2 ) == '>') )

which really doesn't look like what this code probably wants to do.

Attached is a patch to do the more likely 

if( i+2 < rString.Len() &&
   (rString.GetChar( i+1 ) == 'b' || (rString.GetChar( i+1 ) == 'B') &&
   rString.GetChar( +2 ) == '>') )

including the same type of thing later on
Comment 1 caolanm 2008-11-10 17:04:30 UTC 
Created attachment 57843 [details]
what we probably want to do
Comment 2 dtardon 2008-11-11 06:48:04 UTC 
There is another issue that I've found highly suspicious: the

rString.GetChar( +2 ) == '>' 

thing. Shouldn't it be

rString.GetChar( i+2 ) == '>'

instead? I've 'reused' caolan's patch to fix this issue too.
Comment 3 dtardon 2008-11-11 06:50:15 UTC 
Created attachment 57863 [details]
fix of the fix :)
Comment 4 pavel 2008-11-11 07:05:51 UTC 
Ivo's playground :-)
Comment 5 ivo.hinkelmann 2008-11-11 11:37:41 UTC 
hahahaha nice one!

This is used in the readme ... I guess <b> is never ever used in the translation
as the layout is done in the xml code
Comment 6 caolanm 2009-02-27 12:04:10 UTC 
I'll take this, its low-hanging and blocks warnings-free on new compilers
Comment 7 caolanm 2009-02-27 12:06:00 UTC 
done in cmcfixes55
Comment 8 caolanm 2009-03-04 11:17:41 UTC 
Can you code-review these changes in cmcfixes55 and set each to verified if correct
Comment 9 dtardon 2009-03-07 07:31:11 UTC 
Verified in CWS cmcfixes55.
Comment 10 caolanm 2009-03-20 14:20:42 UTC 
closing, is in m44