Fix warnings in networking code

RESOLVED FIXED in mozilla17

Status

()

Core
Networking
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: valentin, Assigned: valentin)

Tracking

unspecified
mozilla17
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
Created attachment 650402 [details] [diff] [review]
Fix v1.0

https://tbpl.mozilla.org/?tree=Try&rev=cbc3e2e6ff8e
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

5 years ago
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.

Updated

5 years ago
Assignee: nobody → valentin.gosu

Comment 4

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

Try run looks fine, requesting review for Valentin.
Attachment #650402 - Flags: review?(jduell.mcbugs)
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).
Attachment #650402 - Flags: review+
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
Attachment #650402 - Flags: review?(jduell.mcbugs)
Sorry for the late reply. I was out.
Josh explained everything quite well. (Thanks, Josh!)
Try is almost complete and mostly green. :)

Comment 8

5 years ago
pushed to mozilla-inbound

http://hg.mozilla.org/integration/mozilla-inbound/rev/e8e5b353ca9c
https://hg.mozilla.org/mozilla-central/rev/e8e5b353ca9c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.