Fix warnings in networking code

RESOLVED FIXED in mozilla17



5 years ago
5 years ago


(Reporter: valentin, Assigned: valentin)



Firefox Tracking Flags

(Not tracked)



(1 attachment)

Comment hidden (empty)

Comment 1

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

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


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, 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/
@@ +2,5 @@
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at */
>  #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.


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)

Comment 7

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