RESOLVED FIXED167242
[WebRTC] libwebrtc headers are incompatible with WebKit compilation flags
https://bugs.webkit.org/show_bug.cgi?id=167242
Summary [WebRTC] libwebrtc headers are incompatible with WebKit compilation flags
youenn fablet
Reported 2017-01-20 10:29:54 PST
Unused parameters for inline functions generate errors in WebKit source code including libwebrtc source code.
Attachments
Patch (29.69 KB, patch)
2017-01-20 10:36 PST, youenn fablet
no flags
Patch for landing (29.77 KB, patch)
2017-01-20 11:53 PST, youenn fablet
no flags
youenn fablet
Comment 1 2017-01-20 10:36:08 PST
WebKit Commit Bot
Comment 2 2017-01-20 10:38:24 PST
Attachment 299355 [details] did not pass style-queue: ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:122: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:123: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:124: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:392: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:393: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:399: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:399: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:413: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:414: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:436: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:436: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:447: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:448: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:448: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:452: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:453: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:453: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:457: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:458: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:458: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:462: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:463: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:463: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:478: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:479: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:482: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:491: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:504: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:521: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:522: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:556: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:556: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:558: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:558: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:561: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:561: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:564: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:564: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:568: Missing space before { [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:568: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:568: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:570: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:570: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:591: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:591: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:594: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectioninterface.h:594: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/base/messagehandler.h:43: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/base/messagehandler.h:43: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/base/messagehandler.h:59: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/base/messagehandler.h:59: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/modules/include/module.h:56: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/modules/include/module.h:56: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/stun.h:220: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/stun.h:220: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectionfactory.h:84: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectionfactory.h:86: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/peerconnectionfactory.h:87: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/port.h:244: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/port.h:245: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/port.h:246: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/port.h:363: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/port.h:363: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:137: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:138: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:138: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:139: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:139: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:177: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:177: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:180: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:180: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:181: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:181: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:184: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:184: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:185: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:185: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:233: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/stunrequest.h:113: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/stunrequest.h:113: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/stunrequest.h:116: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/stunrequest.h:116: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/stunrequest.h:117: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/stunrequest.h:117: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/base/stream.h:135: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/base/stream.h:135: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/base/stream.h:149: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/base/stream.h:149: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/api/jsep.h:101: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtcvideoencoderfactory.h:39: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtcvideoencoderfactory.h:40: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtcvideoencoderfactory.h:41: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtcvideoencoderfactory.h:73: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtcvideoencoderfactory.h:73: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/transport.h:270: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/transport.h:270: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/transport.h:274: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/transport.h:323: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/transport.h:326: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/transport.h:326: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Sou rce/webrtc/media/engine/webrtcvideodecoderfactory.h:35: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/media/base/mediachannel.h:1162: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/media/base/mediachannel.h:1167: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/media/base/mediachannel.h:1168: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/media/base/mediachannel.h:1168: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 106 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Alex Christensen
Comment 3 2017-01-20 11:14:19 PST
Comment on attachment 299355 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=299355&action=review It would probably be worth maintaining a list of changes like https://trac.webkit.org/browser/trunk/Source/ThirdParty/ANGLE/changes.diff so the public and especially libwebrtc developers can see what we need to change from their project. > Source/ThirdParty/libwebrtc/ChangeLog:3 > + [WebRTC] libwebrtc headers are incompatible with WebKit compilation flags Specifically -Wunused-parameter and -Wunused-variable > Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:139 > + const rtc::VideoSinkWants&) override {}; > + void RemoveSink(rtc::VideoSinkInterface<VideoFrame>*) override {}; I don't know if it's worth making these headers match WebKit style. > Source/ThirdParty/libwebrtc/Source/webrtc/base/sanitizer.h:53 > +(void)&(ptr); > +(void)&(element_size); > +(void)&(num_elements); These should be in an #else. Ditto with other changes in this file.
youenn fablet
Comment 4 2017-01-20 11:53:48 PST
Created attachment 299361 [details] Patch for landing
youenn fablet
Comment 5 2017-01-20 11:54:56 PST
(In reply to comment #3) > Comment on attachment 299355 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=299355&action=review > > It would probably be worth maintaining a list of changes like > https://trac.webkit.org/browser/trunk/Source/ThirdParty/ANGLE/changes.diff > so the public and especially libwebrtc developers can see what we need to > change from their project. > > > Source/ThirdParty/libwebrtc/ChangeLog:3 > > + [WebRTC] libwebrtc headers are incompatible with WebKit compilation flags > > Specifically -Wunused-parameter and -Wunused-variable Updated change log. > > > Source/ThirdParty/libwebrtc/Source/webrtc/api/mediastreaminterface.h:139 > > + const rtc::VideoSinkWants&) override {}; > > + void RemoveSink(rtc::VideoSinkInterface<VideoFrame>*) override {}; > > I don't know if it's worth making these headers match WebKit style. We are touching them and making them consistent with other libwebrtc code. I think it is ok. > > > Source/ThirdParty/libwebrtc/Source/webrtc/base/sanitizer.h:53 > > +(void)&(ptr); > > +(void)&(element_size); > > +(void)&(num_elements); > > These should be in an #else. > Ditto with other changes in this file. Updated to put in an #else and added a macro for the 3 parameters
WebKit Commit Bot
Comment 6 2017-01-20 14:31:38 PST
Comment on attachment 299361 [details] Patch for landing Clearing flags on attachment: 299361 Committed r210987: <http://trac.webkit.org/changeset/210987>
WebKit Commit Bot
Comment 7 2017-01-20 14:31:42 PST
All reviewed patches have been landed. Closing bug.
youenn fablet
Comment 8 2017-01-20 14:43:03 PST
> It would probably be worth maintaining a list of changes like > https://trac.webkit.org/browser/trunk/Source/ThirdParty/ANGLE/changes.diff > so the public and especially libwebrtc developers can see what we need to > change from their project. Right, I had one originally checked-in but I changed a bit some patches and it was not worth actually upstreaming the diff. I'll produce one once all libwebrtc patches will be upstreamed.
Alex. Gouaillard
Comment 9 2017-01-26 23:28:49 PST
the logic is wrong in sanitizer, you need to check both preprocessor flags since you are using the defined macro when !RTC_HAS_MSAN Source/webrtc/base/sanitizer.h #if !RTC_HAS_ASAN || !RTC_HAS_MSAN #define SANITIZER_UNUSED3(x, y, z) (void)&(x); \ (void)&(y); \ (void)&(z) #endif caught by google build bots: https://build.chromium.org/p/tryserver.webrtc/builders/linux_asan/builds/21461/steps/compile/logs/stdio
Note You need to log in before you can comment on or make changes to this bug.