984080 - Suppress clang and gcc warnings in third-party code: google/crashreporter, ipc/chromium

Status: RESOLVED FIXED

(Core :: IPC, defect, P4)

Description

[Chris Peterson [:cpeterson]]

Attachment: suppress-google-warnings.patch

ipc/chromium/src/base/logging.cc:15:20 [-Wunused-but-set-variable] variable 'prlevel' set but not used
ipc/chromium/src/base/string_util.cc:307:13 [-Wunused-function] 'bool TrimString(const wstring&, wchar_t const*, std::wstring*)' defined but not used
ipc/chromium/src/base/string_util.cc:313:13 [-Wunused-function] 'bool TrimString(const string&, char const*, std::string*)' defined but not used
ipc/chromium/src/base/string_util.cc:513:27 [-Wtype-limits] comparison of unsigned expression < 0 is always false
ipc/chromium/src/third_party/libevent/evutil_rand.c:62:2 [-Wimplicit-function-declaration] implicit declaration of function 'arc4random_buf'

toolkit/crashreporter/google-breakpad/src/common/dwarf_cu_to_module.h:224:10 [-Wredeclared-class-member] class member cannot be redeclared
toolkit/crashreporter/google-breakpad/src/common/md5.cc:169:3 [-Wdeprecated-register] 'register' storage class specifier is deprecated
toolkit/crashreporter/google-breakpad/src/processor/tokenize.cc:65:7 [-Wlogical-not-parentheses] logical not is only applied to the left hand side of this comparison

Comment 1

[Benjamin Smedberg]

Comment on attachment 8391830 [details] [diff] [review]
suppress-google-warnings.patch

Ted I know I saw some breakpad warnings fixes go in recently: do we still need this or can we just uplift a new breakpad?

for ipc/chromium do we know what it would take to fix the warnings? We forked that code, so we don't need to worry about conflicts with upstream or anything like that.

Comment 2

[Chris Peterson [:cpeterson]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #1)
> Ted I know I saw some breakpad warnings fixes go in recently: do we still
> need this or can we just uplift a new breakpad?

I just reviewed the upstream google-breakpad repo. It looks like all three of the warnings I was suppressing have been fixed upstream. So I will just retract my patch.


(In reply to Benjamin Smedberg  [:bsmedberg] from comment #1)
> for ipc/chromium do we know what it would take to fix the warnings? We
> forked that code, so we don't need to worry about conflicts with upstream or
> anything like that.

The fixes for ipc/chromium warnings would be trivial. If we don't care about upstream conflicts, I'll post a patch to actually fix the ipc/chromium warnings.

Comment 3

[Chris Peterson [:cpeterson]]

Comment on attachment 8391830 [details] [diff] [review]
suppress-google-warnings.patch

This patch is no longer relevant because the breakpad warnings have been fixed upstream and the ipc/chromium warnings can be fixed in our tree.

Comment 4

[Chris Peterson [:cpeterson]]

Attachment: fix-ipc-chromium-warnings.patch

* This patch fixes the following clang and gcc warnings in ipc/chromium (so this code has zero warnings with clang on OS X):

ipc/chromium/src/base/shared_memory_posix.cc:23:12 [-Wunused-const-variable] unused variable 'kSemaphoreSuffix'
ipc/chromium/src/base/string_util.cc:307:13 [-Wunused-function] unused function 'TrimString'
ipc/chromium/src/base/string_util.cc:313:13 [-Wunused-function] unused function 'TrimString'
ipc/chromium/src/base/string_util.cc:513:27 [-Wtype-limits] comparison of unsigned expression < 0 is always false
ipc/chromium/src/base/sys_string_conversions_mac.mm:120:31 [-Wunused-const-variable] unused variable 'kMediumStringEncoding'

* But it does NOT fix the following gcc warnings on Android (because the fixes would be a little more involved and my primary goal is to fix clang warnings on OS X):

ipc/chromium/src/base/logging.cc:15:20 [-Wunused-but-set-variable] variable 'prlevel' set but not used
ipc/chromium/src/third_party/libevent/evutil_rand.c:62:2 [-Wimplicit-function-declaration] implicit declaration of function 'arc4random_buf'


The -Wtype-limits fix (using template struct TestNegT) was backported from Chromium upstream:
https://chromium.googlesource.com/chromium/chromium/+blame/master/base/string_number_conversions.cc

Comment 5

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7bb86a0b7a8d

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7bb86a0b7a8d