Closed
Bug 1190069
Opened 9 years ago
Closed 9 years ago
Uninitialized variable returned in MDNSResponderOperator.cpp
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: kats, Assigned: ma.steiman)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
xeonchen
:
review+
|
Details | Diff | Splinter Review |
As reported at https://groups.google.com/d/msg/mozilla.dev.b2g/ze9HSM6GYIo/YVmM6MpvCwAJ there is an uninitialized variable causing a compile error in some configurations, introduced in bug 1115480
In file included from Unified_cpp_dns_mdns_libmdns0.cpp:2:0:
/home/sfoster/B2G/gecko/netwerk/dns/mdns/libmdns/MDNSResponderOperator.cpp: In member function 'nsresult mozilla::net::MDNSResponderOperator::ResetService(DNSServiceRef)':
/home/sfoster/B2G/gecko/netwerk/dns/mdns/libmdns/MDNSResponderOperator.cpp:291:16: error: 'rv' may be used uninitialized in this function [-Werror=uninitialized]
cc1plus: all warnings being treated as errors
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(xeonchen)
Assignee | ||
Comment 1•9 years ago
|
||
I would love to work on this, I've assigned it to myself.
Assignee: nobody → ma.steiman
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•9 years ago
|
||
I've attached a patch based on Fabrice's email:
https://groups.google.com/forum/#!msg/mozilla.dev.b2g/ze9HSM6GYIo/YVmM6MpvCwAJ
Kartikaya, if you are not a reviewer can you please let me know? I figured you should check this out before submitting it for review, also since you requested info from someone else.
Flags: needinfo?(bugmail.mozilla)
Attachment #8642036 -
Flags: review?(bugmail.mozilla)
Reporter | ||
Comment 3•9 years ago
|
||
Comment on attachment 8642036 [details] [diff] [review]
Fixes uninitialized variable in MDNSResponderOperator.cpp
Review of attachment 8642036 [details] [diff] [review]:
-----------------------------------------------------------------
This is going to call watcher->Init() twice, so unfortunately it's not quite what we want here. But please flag :xeonchen for review, as he should review any changes here. I haven't worked with this code at all.
Attachment #8642036 -
Flags: review?(bugmail.mozilla) → review-
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(bugmail.mozilla)
Assignee | ||
Comment 4•9 years ago
|
||
That's my mistake I jumped the gun based on the email. Would we be able to just declare rv in the if statement similar to:
https://groups.google.com/forum/#!msg/mozilla.dev.b2g/ze9HSM6GYIo/YVmM6MpvCwAJ
>313 if (NS_WARN_IF(NS_FAILED(rv = MDNSResponderOperator::Start()))) {
>314 return rv;
>315 }
then return rv?
Assignee | ||
Comment 5•9 years ago
|
||
The correct link for the above comment is:
https://mxr.mozilla.org/mozilla-central/source/netwerk/dns/mdns/libmdns/MDNSResponderOperator.cpp#313
Assignee | ||
Comment 6•9 years ago
|
||
This patch declares rv inside the if statement. Other parts of the code do the same before returning rv, so please let me know if this will not work. Thanks!!
Attachment #8642036 -
Attachment is obsolete: true
Attachment #8642073 -
Flags: review?(xeonchen)
Comment 7•9 years ago
|
||
Comment on attachment 8642073 [details] [diff] [review]
bug1190069_unititalized_var.diff
Review of attachment 8642073 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM, thank you Kartikaya!
Attachment #8642073 -
Flags: review?(xeonchen) → review+
Comment 8•9 years ago
|
||
(In reply to Gary Chen [:xeonchen] from comment #7)
> Comment on attachment 8642073 [details] [diff] [review]
> bug1190069_unititalized_var.diff
>
> Review of attachment 8642073 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> LGTM, thank you Kartikaya!
Got wrong name, sorry.
Thank you Muhsin!
Flags: needinfo?(xeonchen)
Assignee | ||
Comment 9•9 years ago
|
||
I've changed the status to checkin-needed, please let me know if there's anything else I need to do, thanks!
Flags: needinfo?(xeonchen)
Keywords: checkin-needed
Comment 11•9 years ago
|
||
can we get a try run here ?
Flags: needinfo?(ma.steiman)
Keywords: checkin-needed
Assignee | ||
Comment 12•9 years ago
|
||
Can I get some help on how to do that? I'm pretty new here and haven't done a try run :\
Flags: needinfo?(ma.steiman) → needinfo?(cbook)
This is a very simple change. Just land it.
Keywords: checkin-needed
Comment 14•9 years ago
|
||
Keywords: checkin-needed
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(cbook)
Comment 15•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•