Closed Bug 1168103 Opened 10 years ago Closed 10 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.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: