Closed Bug 1311669 Opened 4 years ago Closed 4 years ago

Run clang-tidy on netwerk/ module

Categories

(Firefox Build System :: Source Code Analysis, defect)

defect
Not set
normal

Tracking

(firefox52 fixed)

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: andi, Assigned: andi)

References

Details

Attachments

(6 files, 2 obsolete files)

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
Attachment #8803287 - Attachment is obsolete: true
Attachment #8803287 - Attachment is obsolete: false
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+
Keywords: leave-open
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
Attachment #8806326 - Flags: review?(valentin.gosu)
Attachment #8806326 - 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

> 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.
Attachment #8805923 - Flags: review?(valentin.gosu)
Attachment #8806326 - Attachment is obsolete: true
(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+
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
MozReview-Commit-ID: B0wDHK1mKGN
Attachment #8807059 - Flags: review?(valentin.gosu)
MozReview-Commit-ID: Ab5BC8NhbIO
Attachment #8807060 - Flags: review?(valentin.gosu)
Attachment #8807059 - Attachment is obsolete: true
Attachment #8807059 - Flags: review?(valentin.gosu)
Attachment #8807060 - Flags: review?(valentin.gosu) → review+
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
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/73a97f8ffeba
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.