Bug 180757

Summary: Enable -Wstrict-prototypes for WebKit
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: WebKit Misc.Assignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, joepeck, mitz, youennf
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=179466
https://bugs.webkit.org/show_bug.cgi?id=180848
https://bugs.webkit.org/show_bug.cgi?id=181146
https://bugs.chromium.org/p/webrtc/issues/detail?id=8984
https://github.com/sctplab/usrsctp/issues/207
Attachments:
Description Flags
Patch v1
none
Patch v2
ews-watchlist: commit-queue-
Archive of layout-test-results from ews117 for mac-elcapitan none

Description David Kilzer (:ddkilzer) 2017-12-13 10:26:26 PST
Enable -Wstrict-prototypes for WebKit to prevent bugs like Bug 179466 from recurring.
Comment 1 David Kilzer (:ddkilzer) 2017-12-13 10:26:41 PST
<rdar://problem/36024132>
Comment 2 David Kilzer (:ddkilzer) 2017-12-14 00:39:56 PST
Created attachment 329333 [details]
Patch v1
Comment 3 EWS Watchlist 2017-12-14 00:41:29 PST
Attachment 329333 [details] did not pass style-queue:


ERROR: Source/WTF/ChangeLog:17:  Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: WTFCrashWithSecurityImplication  [changelog/unwantedsecurityterms] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/common_audio/vad/include/webrtc_vad.h:30:  WebRtcVad_Create is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_processing/ns/noise_suppression_x.h:27:  WebRtcNsx_Create is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_processing/ns/noise_suppression_x.h:107:  WebRtcNsx_num_freq is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_processing/ns/noise_suppression.h:27:  WebRtcNs_Create is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_processing/ns/noise_suppression.h:129:  WebRtcNs_num_freq is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_processing/agc/legacy/gain_control.h:211:  WebRtcAgc_Create is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/common_audio/signal_processing/include/signal_processing_library.h:113:  WebRtcSpl_Init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_coding/codecs/isac/fix/source/codec.h:70:  WebRtcIsacfix_InitTransform is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 9 in 38 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Joseph Pecoraro 2017-12-14 11:13:16 PST
Comment on attachment 329333 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=329333&action=review

rs=me once you get ios/ios-sim building

> Source/ThirdParty/libwebrtc/Source/third_party/usrsctp/usrsctplib/usrsctplib/user_socket.c:399
>  void
> -wakeup_one(ident)
> -	void *ident;
> +wakeup_one(void *ident)

Wow what old syntax!
Comment 5 David Kilzer (:ddkilzer) 2017-12-14 12:32:47 PST
Created attachment 329383 [details]
Patch v2
Comment 6 EWS Watchlist 2017-12-14 12:34:11 PST
Attachment 329383 [details] did not pass style-queue:


ERROR: Source/WTF/ChangeLog:17:  Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: WTFCrashWithSecurityImplication  [changelog/unwantedsecurityterms] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/common_audio/vad/include/webrtc_vad.h:30:  WebRtcVad_Create is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_processing/ns/noise_suppression_x.h:27:  WebRtcNsx_Create is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_processing/ns/noise_suppression_x.h:107:  WebRtcNsx_num_freq is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_processing/ns/noise_suppression.h:27:  WebRtcNs_Create is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_processing/ns/noise_suppression.h:129:  WebRtcNs_num_freq is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_processing/agc/legacy/gain_control.h:211:  WebRtcAgc_Create is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/common_audio/signal_processing/include/signal_processing_library.h:113:  WebRtcSpl_Init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_coding/codecs/isac/fix/source/codec.h:70:  WebRtcIsacfix_InitTransform is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 9 in 38 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 David Kilzer (:ddkilzer) 2017-12-14 16:51:03 PST
(In reply to Build Bot from comment #6)
> Attachment 329383 [details] did not pass style-queue:
> 
> ERROR:
> Source/ThirdParty/libwebrtc/Source/webrtc/common_audio/vad/include/
> webrtc_vad.h:30:  WebRtcVad_Create is incorrectly named. Don't use
> underscores in your identifier names.  [readability/naming/underscores] [4]
> ERROR:
> Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_processing/ns/
> noise_suppression_x.h:27:  WebRtcNsx_Create is incorrectly named. Don't use
> underscores in your identifier names.  [readability/naming/underscores] [4]
> ERROR:
> Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_processing/ns/
> noise_suppression_x.h:107:  WebRtcNsx_num_freq is incorrectly named. Don't
> use underscores in your identifier names.  [readability/naming/underscores]
> [4]
> ERROR:
> Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_processing/ns/
> noise_suppression.h:27:  WebRtcNs_Create is incorrectly named. Don't use
> underscores in your identifier names.  [readability/naming/underscores] [4]
> ERROR:
> Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_processing/ns/
> noise_suppression.h:129:  WebRtcNs_num_freq is incorrectly named. Don't use
> underscores in your identifier names.  [readability/naming/underscores] [4]
> ERROR:
> Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_processing/agc/
> legacy/gain_control.h:211:  WebRtcAgc_Create is incorrectly named. Don't use
> underscores in your identifier names.  [readability/naming/underscores] [4]
> ERROR:
> Source/ThirdParty/libwebrtc/Source/webrtc/common_audio/signal_processing/
> include/signal_processing_library.h:113:  WebRtcSpl_Init is incorrectly
> named. Don't use underscores in your identifier names. 
> [readability/naming/underscores] [4]
> ERROR:
> Source/ThirdParty/libwebrtc/Source/webrtc/modules/audio_coding/codecs/isac/
> fix/source/codec.h:70:  WebRtcIsacfix_InitTransform is incorrectly named.
> Don't use underscores in your identifier names. 
> [readability/naming/underscores] [4]
> Total errors found: 9 in 38 files
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

Bug 180848: check-webkit-style: Stop warning about underscores in webrtc source
https://bugs.webkit.org/show_bug.cgi?id=180848
Comment 8 David Kilzer (:ddkilzer) 2017-12-14 17:00:59 PST
(In reply to Joseph Pecoraro from comment #4)
> Comment on attachment 329333 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=329333&action=review
> 
> rs=me once you get ios/ios-sim building

I'm checking the build against an internal iOS 11.0.x SDK to make sure there aren't any surprises when landing this.

Already checked using Mac OS X 10.12.x.

> > Source/ThirdParty/libwebrtc/Source/third_party/usrsctp/usrsctplib/usrsctplib/user_socket.c:399
> >  void
> > -wakeup_one(ident)
> > -	void *ident;
> > +wakeup_one(void *ident)
> 
> Wow what old syntax!

K&R!
Comment 9 EWS Watchlist 2017-12-14 17:57:51 PST
Comment on attachment 329383 [details]
Patch v2

Attachment 329383 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/5666562

New failing tests:
svg/custom/object-sizing-explicit-height.xhtml
Comment 10 EWS Watchlist 2017-12-14 17:57:52 PST
Created attachment 329432 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 11 David Kilzer (:ddkilzer) 2017-12-14 20:19:52 PST
Committed r225958: <https://trac.webkit.org/changeset/225958>
Comment 12 David Kilzer (:ddkilzer) 2017-12-14 21:23:18 PST
(In reply to David Kilzer (:ddkilzer) from comment #11)
> Committed r225958: <https://trac.webkit.org/changeset/225958>

Build fix for 32-bit:
Committed r225962: < https://trac.webkit.org/changeset/225962>
Comment 13 youenn fablet 2018-03-05 14:36:39 PST
We try to limit the differences between upstream libwebrtc to the minimum since we want to regularly update our own copy. And libwebrtc is still evolving a lot, files get moved all the time.

Ideally, these changes would be upstreamed.
As long as this is not done so, the best approach might be to special case libwebrtc.
I cannot promise to keep them except if EWS bots bring compilation errors.
Comment 14 David Kilzer (:ddkilzer) 2018-03-06 13:10:14 PST
(In reply to youenn fablet from comment #13)
> We try to limit the differences between upstream libwebrtc to the minimum
> since we want to regularly update our own copy. And libwebrtc is still
> evolving a lot, files get moved all the time.
> 
> Ideally, these changes would be upstreamed.

What's the process to get changes upstreamed?  Do we open a bug on crbug.com and post a patch (like ANGLE)?

> As long as this is not done so, the best approach might be to special case
> libwebrtc.
> I cannot promise to keep them except if EWS bots bring compilation errors.

Yep, there will be compilation errors if the changes are reverted.  Easy to re-fix.
Comment 15 David Kilzer (:ddkilzer) 2018-03-06 15:02:43 PST
Related bug to upstream changes to libwebrtc project:
https://bugs.chromium.org/p/webrtc/issues/detail?id=8984