1190069 - Uninitialized variable returned in MDNSResponderOperator.cpp

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

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

Comment 1

[Tyler Steiman]

I would love to work on this, I've assigned it to myself.

Comment 2

[Tyler Steiman]

Attachment: 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.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 4

[Tyler Steiman]

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?

Comment 5

[Tyler Steiman]

The correct link for the above comment is:
https://mxr.mozilla.org/mozilla-central/source/netwerk/dns/mdns/libmdns/MDNSResponderOperator.cpp#313

Comment 6

[Tyler Steiman]

Attachment: 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!!

Comment 7

[Gary Chen [:xeonchen]]

Comment on attachment 8642073 [details] [diff] [review]
bug1190069_unititalized_var.diff

Review of attachment 8642073 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, thank you Kartikaya!

Comment 8

[Gary Chen [:xeonchen]]

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

Comment 9

[Tyler Steiman]

I've changed the status to checkin-needed, please let me know if there's anything else I need to do, thanks!

Comment 10

[Gary Chen [:xeonchen]]

Good job, thank you.

Comment 11

[Carsten Book [:Tomcat]]

can we get a try run here ?

Comment 12

[Tyler Steiman]

Can I get some help on how to do that? I'm pretty new here and haven't done a try run :\

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

This is a very simple change.  Just land it.

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6160dbecbceb

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6160dbecbceb