1311669 - Run clang-tidy on netwerk/ module

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Andi [:andi]]

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 1

[Andi [:andi]]

Attachment: Bug 1311669 - convert loops to range-based loops in C++11.

Review commit: https://reviewboard.mozilla.org/r/87592/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/87592/

Comment 2

[Andi [:andi]]

Attachment: Bug 1311669 - use auto type specifier for variable declarations to improve code readability and maintainability.

Review commit: https://reviewboard.mozilla.org/r/89532/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89532/

Comment 3

[Andi [:andi]]

Attachment: Bug 1311669 - replace default bodies of special member functions with = default;

Review commit: https://reviewboard.mozilla.org/r/89534/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89534/

Comment 4

[Andi [:andi]]

Attachment: Bug 1311669 - replace string literals containing escaped characters with raw string literals.

Review commit: https://reviewboard.mozilla.org/r/89536/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89536/

Comment 5

[Andi [:andi]]

Attachment: Bug 1311669 - Replace integer literals which are cast to bool.

Review commit: https://reviewboard.mozilla.org/r/89538/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89538/

Comment 6

[Andi [:andi]]

Comment on attachment 8803287 [details]
Bug 1311669 - convert loops to range-based loops in C++11.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/87592/diff/1-2/

Comment 7

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

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.

Comment 8

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

Comment on attachment 8805925 [details]
Bug 1311669 - Replace integer literals which are cast to bool.

https://reviewboard.mozilla.org/r/89538/#review89216

Comment 9

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

Comment on attachment 8805924 [details]
Bug 1311669 - replace string literals containing escaped characters with raw string literals.

https://reviewboard.mozilla.org/r/89536/#review89222

Comment 10

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

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

Comment 11

[Andi [:andi]]

Comment on attachment 8803287 [details]
Bug 1311669 - convert loops to range-based loops in C++11.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/87592/diff/2-3/

Comment 12

[Andi [:andi]]

Comment on attachment 8805922 [details]
Bug 1311669 - use auto type specifier for variable declarations to improve code readability and maintainability.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89532/diff/1-2/

Comment 13

[Andi [:andi]]

Comment on attachment 8805923 [details]
Bug 1311669 - replace default bodies of special member functions with = default;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89534/diff/1-2/

Comment 14

[Andi [:andi]]

Comment on attachment 8805924 [details]
Bug 1311669 - replace string literals containing escaped characters with raw string literals.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89536/diff/1-2/

Comment 15

[Andi [:andi]]

Comment on attachment 8805925 [details]
Bug 1311669 - Replace integer literals which are cast to bool.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89538/diff/1-2/

Comment 16

[Andi [:andi]]

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

Comment 17

[Andi [:andi]]

Attachment: Bug 1311669 - Use C++11's override and remove virtual where applicable.

Review commit: https://reviewboard.mozilla.org/r/89800/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89800/

Comment 18

[Chris Peterson [:cpeterson]]

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.

Comment 19

[Andi [:andi]]

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

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

Comment on attachment 8805923 [details]
Bug 1311669 - replace default bodies of special member functions with = default;

https://reviewboard.mozilla.org/r/89534/#review89576

Comment 21

[Pulsebot]

Pushed by [e-mail]:
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

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/81e30b16d0a1
https://hg.mozilla.org/mozilla-central/rev/9c3a2156b10a
https://hg.mozilla.org/mozilla-central/rev/fa0e219113ac
https://hg.mozilla.org/mozilla-central/rev/4ca246ea07bf
https://hg.mozilla.org/mozilla-central/rev/8fe3ca1e5bfc

Comment 23

[Andi [:andi]]

Attachment: Use C++11's override and remove virtual where applicable

MozReview-Commit-ID: B0wDHK1mKGN

Comment 24

[Andi [:andi]]

Attachment: Use C++11's override and remove virtual where applicable

MozReview-Commit-ID: Ab5BC8NhbIO

Comment 25

[Andi [:andi]]

Comment on attachment 8807059 [details] [diff] [review]
Use C++11's override and remove virtual where applicable

also passed try job
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b79250c91058c719d454453adb112c284e9fd7b&selectedJob=30383711

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/73a97f8ffeba
Use C++11's override and remove virtual where applicable. r=valentin

Comment 27

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/73a97f8ffeba