1378683 - Fix incorrectly-written retry loop in nsUnicodeNormalizer

Status: RESOLVED FIXED

(Core :: Internationalization, enhancement)

Description

[Sylvestre Ledru [:Sylvestre]]

Continue; in https://dxr.mozilla.org/mozilla-central/source/intl/unicharutil/nsUnicodeNormalizer_ICU.cpp?q=nsUnicodeNormalizer_ICU.cpp&redirect_type=direct#44 is misleading as we are already at the end of the loop

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1378683 - Remove a useless continue; declaration

Review commit: https://reviewboard.mozilla.org/r/154832/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/154832/

Comment 2

[Jonathan Kew [:jfkthame]]

Comment on attachment 8883864 [details]
Bug 1378683 - Remove a useless continue; declaration

https://reviewboard.mozilla.org/r/154832/#review159844

No, `continue` is needed here to cause the loop to repeat; otherwise it will terminate. (It's normally a single-iteration "loop", using do...while(false); the exceptional case is when we need to re-do the conversion after enlarging the target buffer, in which case we explicitly use `continue` to repeat the block.)

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Looks like coverity disagrees with you:

"continue_in_do_while_false: A continue statement within a do-while loop only continues execution of the loop body if the loop continuation condition is still true. Since the condition will never be true in a "do ... while (false);" loop the continue statement has the same effect as a break statement. Did you intend execution to continue at the top of the loop?"

Comment 4

[Jonathan Kew [:jfkthame]]

Huh, interesting... I guess that's a gotcha I hadn't noticed! OK, in that case this code doesn't work as intended (although it was meant to be handling a rare case, so the real-world impact is probably small-to-nonexistent), and we should fix it accordingly.

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: Fix incorrectly-written retry loop in nsUnicodeNormalizer

Something like this should make more sense.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Good thing with CS: every day, we can still learn something new ;)

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8883879 [details] [diff] [review]
Fix incorrectly-written retry loop in nsUnicodeNormalizer

Seems to be better but I am no expert in that code ;)

Comment 8

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/23ab5a681338e2b4e516ad1f37a184018dce18e7
Bug 1378683 - Fix incorrectly-written retry loop in nsUnicodeNormalizer. r=sylvestre

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/23ab5a681338
Fix incorrectly-written retry loop in nsUnicodeNormalizer. r=sylvestre

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/23ab5a681338