Closed Bug 1168103 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: Sylvestre, Assigned: m.jethva01, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: [good first bug][lang=C++][CID 1274591])

Attachments

(2 files)

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.
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 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).
Assignee: nobody → m.jethva01
Hi, Once I am back home, I will upload the new patch asap and submit for review. Thanks
Attached patch patch.diffSplinter Review
Removed the comment
Comment on attachment 8611913 [details] [diff] [review]
patch.diff

Hi, I updated the line since value is already defined based off the IF statement.
Attachment #8611913 - Flags: review?(philringnalda)
Attachment #8611913 - Flags: review?(philringnalda) → review?(sledru)
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.
Attachment #8611913 - Flags: review?(sledru) → review?(terrence)
Oh. I did git blame because I didn't have mercurial installed. I will install it tonight.
Comment on attachment 8611913 [details] [diff] [review]
patch.diff

According to mercurial, evilpie touched this code most recently.
Attachment #8611913 - Flags: review?(terrence) → review?(evilpies)
Attachment #8611913 - Flags: review?(evilpies) → review+
How can I test this on the try server?
Keywords: checkin-needed
I don't think this is needed. The patch is simple enough!
:D
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.
https://hg.mozilla.org/mozilla-central/rev/623f19585de1
https://hg.mozilla.org/mozilla-central/rev/077e8ebaae14
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.