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)
Core
JavaScript Engine
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)
733 bytes,
patch
|
Details | Diff | Splinter Review | |
645 bytes,
patch
|
evilpie
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•8 years ago
|
||
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, '
Assignee | ||
Comment 2•8 years ago
|
||
Reporter | ||
Comment 3•8 years ago
|
||
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).
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → m.jethva01
Assignee | ||
Comment 4•8 years ago
|
||
Hi, Once I am back home, I will upload the new patch asap and submit for review. Thanks
Assignee | ||
Comment 5•8 years ago
|
||
Removed the comment
Assignee | ||
Comment 6•8 years ago
|
||
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)
Updated•8 years ago
|
Attachment #8611913 -
Flags: review?(philringnalda) → review?(sledru)
Reporter | ||
Comment 7•8 years ago
|
||
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)
Assignee | ||
Comment 8•8 years ago
|
||
Oh. I did git blame because I didn't have mercurial installed. I will install it tonight.
Comment 9•8 years ago
|
||
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)
Updated•8 years ago
|
Attachment #8611913 -
Flags: review?(evilpies) → review+
Assignee | ||
Comment 10•8 years ago
|
||
How can I test this on the try server?
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 11•8 years ago
|
||
I don't think this is needed. The patch is simple enough!
Assignee | ||
Comment 12•8 years ago
|
||
:D
Comment 13•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/623f19585de1
Keywords: checkin-needed
Comment 15•8 years ago
|
||
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•8 years ago
|
||
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
Reporter | ||
Updated•5 years ago
|
Blocks: coverity-analysis
You need to log in
before you can comment on or make changes to this bug.
Description
•