Run clang-tidy on netwerk/ module

RESOLVED FIXED in Firefox 52

Status

defect
RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: andi, Assigned: andi)

Tracking

Trunk
mozilla52

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(6 attachments, 2 obsolete attachments)

(Assignee)

Description

3 years ago
This bug is based on the work of Sylvestre that started: https://bugzilla.mozilla.org/show_bug.cgi?id=1302401

Using the checkers provided by clang-tidy we can refresh the code to make it use the features of C++11 like:

- auto variables declarations
- default CTORS and DTORS
- using new range loop operators
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

3 years ago
Attachment #8803287 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8803287 - Attachment is obsolete: false
(Assignee)

Updated

3 years ago
Attachment #8803287 - Flags: review?(valentin.gosu)
Attachment #8805922 - Flags: review?(valentin.gosu)
Attachment #8805924 - Flags: review?(valentin.gosu)
Attachment #8805925 - Flags: review?(valentin.gosu)
Comment on attachment 8803287 [details]
Bug 1311669 - convert loops to range-based loops in C++11.

https://reviewboard.mozilla.org/r/87592/#review89210

::: netwerk/wifi/osx_corewlan.mm:62
(Diff revision 2)
>  
>        // [CWInterface bssidData] is deprecated on OS X 10.7 and up.  Which is
>        // is a pain, so we'll use it for as long as it's available.
>        unsigned char macData[6] = {0};
>        if ([anObject respondsToSelector:@selector(bssidData)]) {
> -        NSData* data = [anObject bssidData];
> +        NSData* data = [anObject ssidData];

Is this a typo? If it's intentional, please explain the reason for it, as I'm not very familiar with the OSX APIs.
Attachment #8803287 - Flags: review?(valentin.gosu) → review+
Comment on attachment 8805925 [details]
Bug 1311669 - Replace integer literals which are cast to bool.

https://reviewboard.mozilla.org/r/89538/#review89216
Attachment #8805925 - Flags: review?(valentin.gosu) → review+
Comment on attachment 8805924 [details]
Bug 1311669 - replace string literals containing escaped characters with raw string literals.

https://reviewboard.mozilla.org/r/89536/#review89222
Attachment #8805924 - Flags: review?(valentin.gosu) → review+
Comment on attachment 8805922 [details]
Bug 1311669 - use auto type specifier for variable declarations to improve code readability and maintainability.

https://reviewboard.mozilla.org/r/89532/#review89226
Attachment #8805922 - Flags: review?(valentin.gosu) → review+
(Assignee)

Updated

3 years ago
Keywords: leave-open
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 16

3 years ago
mozreview-review-reply
Comment on attachment 8803287 [details]
Bug 1311669 - convert loops to range-based loops in C++11.

https://reviewboard.mozilla.org/r/87592/#review89210

> Is this a typo? If it's intentional, please explain the reason for it, as I'm not very familiar with the OSX APIs.

After giving it a bit of more thought i don't think we should mofigy mm files, specially in this case, clang-tidy was smat doing this modification since bssidData is no longer a member of CWNetwork, but still i don't want to break compatibility with osx < 10.7
(Assignee)

Updated

3 years ago
Attachment #8806326 - Flags: review?(valentin.gosu)
(Assignee)

Updated

3 years ago
Attachment #8806326 - Flags: review?(valentin.gosu)

Comment 18

3 years ago
mozreview-review-reply
Comment on attachment 8803287 [details]
Bug 1311669 - convert loops to range-based loops in C++11.

https://reviewboard.mozilla.org/r/87592/#review89210

> After giving it a bit of more thought i don't think we should mofigy mm files, specially in this case, clang-tidy was smat doing this modification since bssidData is no longer a member of CWNetwork, but still i don't want to break compatibility with osx < 10.7

bssidData is the wireless MAC address. ssidData is the human-readable wireless network name. These methods are not interchangeable.
(Assignee)

Updated

3 years ago
Attachment #8805923 - Flags: review?(valentin.gosu)
(Assignee)

Updated

3 years ago
Attachment #8806326 - Attachment is obsolete: true
(Assignee)

Comment 19

3 years ago
(In reply to Chris Peterson [:cpeterson] from comment #18)
> Comment on attachment 8803287 [details]
> Bug 1311669 - convert loops to range-based loops in C++11.
> 
> https://reviewboard.mozilla.org/r/87592/#review89210
> 
> > After giving it a bit of more thought i don't think we should mofigy mm files, specially in this case, clang-tidy was smat doing this modification since bssidData is no longer a member of CWNetwork, but still i don't want to break compatibility with osx < 10.7
> 
> bssidData is the wireless MAC address. ssidData is the human-readable
> wireless network name. These methods are not interchangeable.

Thanks Chris for clearing this out.
Comment on attachment 8805923 [details]
Bug 1311669 - replace default bodies of special member functions with = default;

https://reviewboard.mozilla.org/r/89534/#review89576
Attachment #8805923 - Flags: review?(valentin.gosu) → review+

Comment 21

3 years ago
Pushed by bpostelnicu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/81e30b16d0a1
convert loops to range-based loops in C++11. r=valentin
https://hg.mozilla.org/integration/autoland/rev/9c3a2156b10a
use auto type specifier for variable declarations to improve code readability and maintainability. r=valentin
https://hg.mozilla.org/integration/autoland/rev/fa0e219113ac
replace default bodies of special member functions with = default; r=valentin
https://hg.mozilla.org/integration/autoland/rev/4ca246ea07bf
replace string literals containing escaped characters with raw string literals. r=valentin
https://hg.mozilla.org/integration/autoland/rev/8fe3ca1e5bfc
Replace integer literals which are cast to bool. r=valentin
(Assignee)

Comment 23

3 years ago
MozReview-Commit-ID: B0wDHK1mKGN
Attachment #8807059 - Flags: review?(valentin.gosu)
(Assignee)

Comment 24

3 years ago
MozReview-Commit-ID: Ab5BC8NhbIO
Attachment #8807060 - Flags: review?(valentin.gosu)
(Assignee)

Updated

3 years ago
Attachment #8807059 - Attachment is obsolete: true
Attachment #8807059 - Flags: review?(valentin.gosu)
Attachment #8807060 - Flags: review?(valentin.gosu) → review+

Comment 26

3 years ago
Pushed by bpostelnicu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/73a97f8ffeba
Use C++11's override and remove virtual where applicable. r=valentin
(Assignee)

Updated

3 years ago
Keywords: leave-open

Comment 27

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/73a97f8ffeba
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52

Updated

a year ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.