Closed Bug 1273480 Opened 4 years ago Closed 4 years ago

[Static Analysis][Logically dead code] In function UnescapeSubstr

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: andi, Assigned: andi)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: CID 1361611)

Attachments

(1 file)

The Static Analysis tool Coverity added that variable |status| does not change in the following context: 

>>                    if (code < 0x10000) {
>>                        status = buf.append((char16_t)code);
>>                    } else {
>>                        status = status && buf.append((char16_t)((code - 0x10000) / 1024 + 0xD800));
>>                        status = status && buf.append((char16_t)(((code - 0x10000) % 1024) + 0xDC00));

This is because prior to usage status is initialized with false:

>>            bool status = false;

I think the correct statement for status is:

>>                    } else {
>>                        status = buf.append((char16_t)((code - 0x10000) / 1024 + 0xD800)) &&
>>                                 buf.append((char16_t)(((code - 0x10000) % 1024) + 0xDC00))
>>                    }
Attachment #8753342 - Flags: review?(jorendorff) → review+
Comment on attachment 8753342 [details]
MozReview Request: Bug 1273480 - avoid dead code on |status| assignment. r?jorendorff

https://reviewboard.mozilla.org/r/53222/#review49984

::: js/src/jsfun.cpp:1550
(Diff revision 1)
>                      } else {
> -                        status = status && buf.append((char16_t)((code - 0x10000) / 1024 + 0xD800));
> -                        status = status && buf.append((char16_t)(((code - 0x10000) % 1024) + 0xDC00));
> +                        status = buf.append((char16_t)((code - 0x10000) / 1024 
> +                            + 0xD800)) 
> +                            &&
> +                            buf.append((char16_t)(((code - 0x10000) % 1024) 
> +                            + 0xDC00));

Please delete the trailing spaces on these lines.

We allow code out to 99 columns in js/src, so please don't squish it like this! :)

Thanks for the ptach.
Comment on attachment 8753342 [details]
MozReview Request: Bug 1273480 - avoid dead code on |status| assignment. r?jorendorff

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53222/diff/1-2/
https://hg.mozilla.org/mozilla-central/rev/c844767132fe
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.