Uninitialized variable returned in MDNSResponderOperator.cpp

RESOLVED FIXED in Firefox 42

Status

()

Core
DOM
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: kats, Assigned: Tyler Steiman)

Tracking

unspecified
mozilla42
Points:
---

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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
Flags: needinfo?(xeonchen)
(Assignee)

Comment 1

3 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

3 years ago
Created attachment 8642036 [details] [diff] [review]
Fixes uninitialized variable in MDNSResponderOperator.cpp

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)
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-
Flags: needinfo?(bugmail.mozilla)
(Assignee)

Comment 4

3 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 6

3 years ago
Created attachment 8642073 [details] [diff] [review]
bug1190069_unititalized_var.diff

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 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+
(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

3 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
Good job, thank you.
Flags: needinfo?(xeonchen)
can we get a try run here ?
Flags: needinfo?(ma.steiman)
Keywords: checkin-needed
(Assignee)

Comment 12

3 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
(Assignee)

Updated

3 years ago
Flags: needinfo?(cbook)
https://hg.mozilla.org/mozilla-central/rev/6160dbecbceb
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.