781399 - Fix warnings in networking code

Status: RESOLVED FIXED

(Core :: Networking, defect)

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Attachment: Fix v1.0

https://tbpl.mozilla.org/?tree=Try&rev=cbc3e2e6ff8e

Comment 2

[Jason Duell]

Comment on attachment 650402 [details] [diff] [review]
Fix v1.0

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

Valentin:

I assume you want this reviewed?  If so it's usually a good idea to assign it to one of the necko module peers (list at https://wiki.mozilla.org/Modules/Core#Necko), else it will get forgotten.

I need more info to understand whether this patch actually gets rid of warnings--see below.  Also, you could probably mark this bug RESOLVED DUPLICATE of bug 745296 and add any more patches we need to get rid of necko warnings there.

::: netwerk/base/src/nsFileStreams.h
@@ +175,5 @@
>  class nsPartialFileInputStream : public nsFileInputStream,
>                                   public nsIPartialFileInputStream
>  {
>  public:
> +    using nsFileInputStream::Init;

What warning does this get rid of?  I'm used to this sort of thing causing a compiler error rather than a warning...

::: netwerk/wifi/osx_corewlan.mm
@@ +2,5 @@
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  #import <Cocoa/Cocoa.h>
> +#import <CoreWLAN/CoreWLAN.h>

Same here--I'd expect an #include to get rid of an error, but rarely fix a warning.

Comment 3

[Josh Aas]

I suggested to Valentin that he wait for a green try run before requesting review.

I can confirm that these fixes do fix warnings, at least on Mac OS X with clang.

The first one is a hidden-via-overload warning for the "Init" symbol. There are methods named that in nsFileINputStream and nsIPartialFileInputStream, though with different signatures. What he did un-hides the symbol and the warning goes away.

As for the second one... An Obj-C method call (really a message) to a method that doesn't necessarily exist isn't fatal at compile-time. It's just a warning, basically because it's a message not a function call.  In this case, the methods in question weren't publicly exposed on OS X 10.5 so we just made the calls anyway and they worked. Now that we don't support 10.5 we can depend on the CoreWLAN framework headers existing (10.6+), so we can just include them and kill off the warnings.

I hope this is a clear enough explanation! I may be typing a bit too quickly atm.

Comment 4

[Josh Aas]

Comment on attachment 650402 [details] [diff] [review]
Fix v1.0

Try run looks fine, requesting review for Valentin.

Comment 5

[Jason Duell]

Comment on attachment 650402 [details] [diff] [review]
Fix v1.0

Sounds good.  +r assuming try is green (good luck getting try results in a web browser--I haven't been able to for last day).

Comment 6

[Jason Duell]

Comment on attachment 650402 [details] [diff] [review]
Fix v1.0

heh--this is what I get when I try to "discard" Josh's changes that he made while I was marking +r

Comment 7

[Valentin Gosu [:valentin] (he/him)]

Sorry for the late reply. I was out.
Josh explained everything quite well. (Thanks, Josh!)
Try is almost complete and mostly green. :)

Comment 8

[Josh Aas]

pushed to mozilla-inbound

http://hg.mozilla.org/integration/mozilla-inbound/rev/e8e5b353ca9c

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e8e5b353ca9c