Fix incorrectly-written retry loop in nsUnicodeNormalizer

RESOLVED FIXED in Firefox 56

Status

()

RESOLVED FIXED
a year ago
3 months ago

People

(Reporter: sylvestre, Assigned: jfkthame)

Tracking

(Blocks: 1 bug, {coverity})

unspecified
mozilla56
coverity
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [CID 1402705])

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (mozreview-request)
(Reporter)

Updated

a year ago
Assignee: nobody → sledru
(Assignee)

Comment 2

a year ago
mozreview-review
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.)
Attachment #8883864 - Flags: review-
(Reporter)

Comment 3

a year ago
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?"
(Assignee)

Comment 4

a year ago
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.
(Assignee)

Comment 5

a year ago
Created attachment 8883879 [details] [diff] [review]
Fix incorrectly-written retry loop in nsUnicodeNormalizer

Something like this should make more sense.
Attachment #8883879 - Flags: review?(sledru)
(Reporter)

Comment 6

a year ago
Good thing with CS: every day, we can still learn something new ;)
Assignee: sledru → jfkthame
(Reporter)

Comment 7

a year ago
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 ;)
Attachment #8883879 - Flags: review?(sledru) → review+
(Reporter)

Updated

a year ago
Summary: Remove a useless continue; declaration → Fix incorrectly-written retry loop in nsUnicodeNormalizer
(Assignee)

Comment 8

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

Comment 9

a year ago
Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/23ab5a681338
Fix incorrectly-written retry loop in nsUnicodeNormalizer. r=sylvestre

Comment 10

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/23ab5a681338
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
(Reporter)

Updated

a year ago
Attachment #8883864 - Attachment is obsolete: true
(Reporter)

Updated

3 months ago
Blocks: 1230156
You need to log in before you can comment on or make changes to this bug.