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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andi [:andi]]

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))
>>                    }

Comment 1

[Andi [:andi]]

Attachment: MozReview Request: Bug 1273480 - avoid dead code on |status| assignment. r?jorendorff

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

Comment 2

[Jason Orendorff [:jorendorff]]

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 3

[Andi [:andi]]

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/

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c844767132fe

Comment 5

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

https://hg.mozilla.org/mozilla-central/rev/c844767132fe