1168103 - Execution cannot reach the expression "?unknown?" inside this statement in jsfriendapi.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

In https://dxr.mozilla.org/mozilla-central/source/js/src/jsfriendapi.cpp?from=%20%20%20%20jsfriendapi.cpp#782
This expression is useless:
value ? value : "?unknown?",
As we already check that 'value' is not NULL a few lines upper.

Comment 1

[Mayank Jethva]

Hi, I would like to fix this bug. I have cloned the repository and fixed the source code.

I just replaced line 782 as such:   ' value ? value : "?unknown?", '  => ' value, '

Comment 2

[Mayank Jethva]

Attachment: Replacing line with non needed conditional statement

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8611010 [details] [diff] [review]
Replacing line with non needed conditional statement

Please remove the comment. It is not necessary.

Once you uploaded the new version of the patch, please submit for review (for example: look at the hg history of the file to know who is reviewing this file).

Comment 4

[Mayank Jethva]

Hi, Once I am back home, I will upload the new patch asap and submit for review. Thanks

Comment 5

[Mayank Jethva]

Attachment: patch.diff

Removed the comment

Comment 6

[Mayank Jethva]

Comment on attachment 8611913 [details] [diff] [review]
patch.diff

Hi, I updated the line since value is already defined based off the IF statement.

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8611913 [details] [diff] [review]
patch.diff

Mayank, Phil is a sheriff (folks who are in charge of managing the commit and uplift of the patches). 
Terrence is probably a better reviewer.

Comment 8

[Mayank Jethva]

Oh. I did git blame because I didn't have mercurial installed. I will install it tonight.

Comment 9

[Terrence Cole [:terrence]]

Comment on attachment 8611913 [details] [diff] [review]
patch.diff

According to mercurial, evilpie touched this code most recently.

Comment 10

[Mayank Jethva]

How can I test this on the try server?

Comment 11

[Sylvestre Ledru [:Sylvestre]]

I don't think this is needed. The patch is simple enough!

Comment 12

[Mayank Jethva]

:D

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/623f19585de1

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/077e8ebaae14

Comment 15

[Tom Schuster [:evilpie]]

I just noticed that you used tabs instead of spaces. js/src uses 4 spaces for indention, the rest of Gecko usually uses 2 spaces, but never tabs.

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/623f19585de1
https://hg.mozilla.org/mozilla-central/rev/077e8ebaae14