Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1080)

Issue 1611763002: exploitability_unittest: fix warnings (Closed)

Created:
4 years, 11 months ago by vapier
Modified:
4 years, 11 months ago
Reviewers:
Ted Mielczarek, ivanpe
CC:
google-breakpad-dev_googlegroups.com
Base URL:
https://chromium.googlesource.com/breakpad/breakpad.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

exploitability_unittest: fix warnings The std::getline function always returns its first arg (which is an iostream object) and cannot return anything else. Thus, testing its value is pointless, and even leads to build errors w/at least gcc-5 due to gtest ASSERT_TRUE funcs only taking bool types: .../exploitability_unittest.cc: In member function 'virtual void {anonymous}::ExploitabilityLinuxUtilsTest_DisassembleBytesTest_Test::TestBody()': .../exploitability_unittest.cc:200:136: error: no matching function for call to 'testing::AssertionResult::AssertionResult(std::basic_istream<char>&)' In file included from .../breakpad_googletest_includes.h:33:0, from .../exploitability_unittest.cc:35: .../gtest.h:262:12: note: candidate: testing::AssertionResult::AssertionResult(bool) Since we know this never fails, simply drop the ASSERT_TRUE usage. The next line already checks the content of the buffer we read. Further on in the file, we hit some signed warnings: In file included from .../breakpad_googletest_includes.h:33:0, from .../exploitability_unittest.cc:35: .../gtest.h: In instantiation of 'testing::AssertionResult testing::internal::CmpHelperEQ(const char*, const char*, const T1&, const T2&) [with T1 = long unsigned int; T2 = int]': .../gtest.h:1484:23: required from 'static testing::AssertionResult testing::internal::EqHelper<lhs_is_null_literal>::Compare(const char*, const char*, const T1&, const T2&) [with T1 = long unsigned int; T2 = int; bool lhs_is_null_literal = false]' .../exploitability_unittest.cc:241:289: required from here .../gtest.h:1448:16: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare] if (expected == actual) { This is because we compare the register value (a uint64_t) directly to an integer constant, and those are signed by default. Stick a U suffix on them to fix things up. BUG=chromium:579384 TEST=`make check` passes R=ivanpe@chromium.org Committed: https://chromium.googlesource.com/breakpad/breakpad/+/585ba07db02da99787ec764809aa180a624e7a1a

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -7 lines) Patch
M src/processor/exploitability_unittest.cc View 2 chunks +6 lines, -7 lines 0 comments Download

Messages

Total messages: 5 (2 generated)
vapier
4 years, 11 months ago (2016-01-20 23:45:18 UTC) #2
ivanpe
LGTM
4 years, 11 months ago (2016-01-21 00:39:45 UTC) #3
vapier
4 years, 11 months ago (2016-01-21 05:50:36 UTC) #5
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as
585ba07db02da99787ec764809aa180a624e7a1a (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698