16 Commits

Author SHA1 Message Date
Mark Mentovai
cc166d71f4 Use base::size where appropriate, and ArraySize elsewhere
This is a follow-up to c8a016b99d97, following the post-landing
discussion at
https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1393921/5#message-2058541d8c4505d20a990ab7734cd758e437a5f7

base::size, and std::size that will eventually replace it when C++17 is
assured, does not allow the size of non-static data members to be taken
in constant expression context. The remaining uses of ArraySize are in:

minidump/minidump_exception_writer.cc (×1)
minidump/minidump_system_info_writer.cc (×2, also uses base::size)
snapshot/cpu_context.cc (×4, also uses base::size)
util/misc/arraysize_test.cc (×10, of course)

The first of these occurs when initializing a constexpr variable. All
others are in expressions used with static_assert.

Includes:
Update mini_chromium to 737433ebade4d446643c6c07daae02a67e8deccao

f701716d9546 Add Windows ARM64 build target to mini_chromium
87a95a3d6ac2 Remove the arraysize macro
1f7255ead1f7 Placate MSVC in areas of base::size usage
737433ebade4 Add cast

Bug: chromium:837308
Change-Id: I6a5162654461b1bdd9b7b6864d0d71a734bcde19
Reviewed-on: https://chromium-review.googlesource.com/c/1396108
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-01-04 22:42:57 +00:00
Avi Drissman
c8a016b99d Remove base's arraysize from Crashpad.
BUG=837308
R=mark@chromium.org

Change-Id: Ibecbfc7bc2d61ee54bc1114e4b20978adbc77db2
Reviewed-on: https://chromium-review.googlesource.com/c/1393921
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
2019-01-03 19:44:15 +00:00
Scott Graham
b83f4c731d Let UUID::InitializeFromString accept StringPiece16 too
Split out of crrev.com/c/689745 by jperaza, with a simple test added.

It is useful for this to be an overload instead of a separate signature
so that code that extracts a UUID string out of a filename can treat it
generically between Windows and non-Windows.

Bug: crashpad:196, crashpad:206
Change-Id: I0d7d84a93d9526d1aae8839179dfe903acca091b
Reviewed-on: https://chromium-review.googlesource.com/916885
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2018-02-13 20:18:24 +00:00
Mark Mentovai
281be63d00 Standardize on static constexpr for arrays when possible
This uses “static” at function scope to avoid making local copies, even
in cases where the compiler can’t see that the local copy is
unnecessary. “constexpr” adds additional safety in that it prevents
global state from being initialized from any runtime dependencies, which
would be undesirable.

At namespace scope, “constexpr” is also used where appropriate.

For the most part, this was a mechanical transformation for things
matching '(^| )const [^=]*\['.

Similar transformations could be applied to non-arrays in some cases,
but there’s limited practical impact in most non-array cases relative to
arrays, there are far more use sites, and much more manual intervention
would be required.

Change-Id: I3513b739ee8b0be026f8285475cddc5f9cc81152
Reviewed-on: https://chromium-review.googlesource.com/583997
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-07-25 17:40:51 +00:00
Mark Mentovai
4b450c8137 test: Use (actual, [un]expected) in gtest {ASSERT,EXPECT}_{EQ,NE}
gtest used to require (expected, actual) ordering for arguments to
EXPECT_EQ and ASSERT_EQ, and in failed test assertions would identify
each side as “expected” or “actual.” Tests in Crashpad adhered to this
traditional ordering. After a gtest change in February 2016, it is now
agnostic with respect to the order of these arguments.

This change mechanically updates all uses of these macros to (actual,
expected) by reversing them. This provides consistency with our use of
the logging CHECK_EQ and DCHECK_EQ macros, and makes for better
readability by ordinary native speakers. The rough (but working!)
conversion tool is
https://chromium-review.googlesource.com/c/466727/1/rewrite_expectassert_eq.py,
and “git cl format” cleaned up its output.

EXPECT_NE and ASSERT_NE never had a preferred ordering. gtest never made
a judgment that one side or the other needed to provide an “unexpected”
value. Consequently, some code used (unexpected, actual) while other
code used (actual, unexpected). For consistency with the new EXPECT_EQ
and ASSERT_EQ usage, as well as consistency with CHECK_NE and DCHECK_NE,
this change also updates these use sites to (actual, unexpected) where
one side can be called “unexpected” as, for example, std::string::npos
can be. Unfortunately, this portion was a manual conversion.

References:

https://github.com/google/googletest/blob/master/googletest/docs/Primer.md#binary-comparison
77d6b17338
https://github.com/google/googletest/pull/713

Change-Id: I978fef7c94183b8b1ef63f12f5ab4d6693626be3
Reviewed-on: https://chromium-review.googlesource.com/466727
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-04-04 12:34:24 +00:00
Mark Mentovai
57b2210ed7 GCC fix: Make UUID POD
This eliminates all constructors, but nearly all points of use were
using the default constructor to initialize a UUID member variable as in
uuid_(). This syntax will still produce a zeroed-out UUID.

While compiling, for example, minidump_rva_list_writer.cc:

In file included from ../../minidump/minidump_rva_list_writer.h:25:0,
                 from ../../minidump/minidump_rva_list_writer.cc:15:
../../minidump/minidump_extensions.h:412:8: error: ignoring packed attribute because of unpacked non-POD field ‘crashpad::UUID crashpad::MinidumpCrashpadInfo::report_id’ [-Werror]
   UUID report_id;
        ^~~~~~~~~
../../minidump/minidump_extensions.h:424:8: error: ignoring packed attribute because of unpacked non-POD field ‘crashpad::UUID crashpad::MinidumpCrashpadInfo::client_id’ [-Werror]
   UUID client_id;
        ^~~~~~~~~

Tested with:
 - GCC 4.9 from NDK r13 targeting arm with SDK 16
 - GCC 4.9 from NDK r13 targeting arm64 with SDK 21
 - GCC 6.2 targeting x86_64

BUG=crashpad:30

Change-Id: Iec6b1557441b69d75246f2f75c59c4158fb7ca29
Reviewed-on: https://chromium-review.googlesource.com/409641
Reviewed-by: Scott Graham <scottmg@chromium.org>
2016-11-11 17:37:14 +00:00
Mark Mentovai
6d2d31d2d1 Use base/macros.h instead of base/basictypes.h
This was done in Chromium’s local copy of Crashpad in 562827afb599. This
change is similar to that one, except more care was taken to avoid
including headers from a .cc or _test.cc when already included by the
associated .h. Rather than using <stddef.h> for size_t, Crashpad has
always used <sys/types.h>, so that’s used here as well.

This updates mini_chromium to 8a2363f486e3a0dc562a68884832d06d28d38dcc,
which removes base/basictypes.h.

e128dcf10122 Remove base/move.h; use std::move() instead of Pass()
8a2363f486e3 Move basictypes.h to macros.h

R=avi@chromium.org

Review URL: https://codereview.chromium.org/1566713002 .
2016-01-06 12:22:50 -05:00
Scott Graham
8272a4cefe Use LockFile/UnlockFile for Settings to port to Windows
Adds LoggingOpenFileForReadAndWrite, LoggingTruncateFile, and UUID::GenerateFromSystem
in support.

R=mark@chromium.org, rsesek@chromium.org
BUG=crashpad:13

Review URL: https://codereview.chromium.org/999953002
2015-04-20 14:21:48 -07:00
Mark Mentovai
e7b80a52f5 win: Add UUID::InitializeFromSystemUUID().
The new call is also used in
CrashReportDatabaseWin::PrepareNewCrashReport(). Previously, that method
used the UUID::InitializeFromBytes() constructor. That actually caused
various fields of the UUID to be byte-swapped so that the ::UUID and
crashpad::UUID would be different UUIDs. Although a UUID is mostly
random, the version field in data_3 is used as a namespace and should be
4 for random UUIDs, and this was not the case under swapping.

TEST=crashpad_util_test UUID.FromSystem
BUG=crashpad:1
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1004913004
2015-03-13 13:53:38 -04:00
Scott Graham
cb8c01f410 win: Some %zu to PRIuS
%zu aborts in system printf functions on Windows, so use PRIuS instead.

R=mark@chromium.org, rsesek@chromium.org
BUG=crashpad:1

Review URL: https://codereview.chromium.org/849193002
2015-01-15 10:00:43 -08:00
Robert Sesek
c75dc46b17 Add UUID::InitializeFromString().
R=mark@chromium.org
TEST=util_test --gtest_filter=UUID.FromString

Review URL: https://codereview.chromium.org/820783004
2015-01-02 18:46:10 -05:00
Mark Mentovai
6d1af6922f Don’t use using directives (“using namespace”) in tests.
The contents of tests are moved into the namespace
crashpad::test::(anonymous namespace).

https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Namespaces

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/635883002
2014-10-07 17:28:50 -04:00
Mark Mentovai
4f74716f6d Add MachOImageReader and its test, which also tests
MachOImageSegmentReader.

TEST=util_test MachOImageReader.*
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/535343004
2014-09-04 11:45:40 -04:00
Mark Mentovai
4ae57690fa Put back the standard-layout checks lost in 5bbb7e8abbe0.
TEST=util_test UUID.UUID
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/440603002
2014-08-03 18:50:09 -04:00
Mark Mentovai
5bbb7e8abb Fix the UUID structure to have distinct fields, as expected by
Breakpad’s minidump reader.

TEST=util_test UUID.UUID
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/436933002
2014-08-01 16:10:58 -04:00
Mark Mentovai
4d6b867a1f Add UUID, c16lcpy (strlcpy for char16*), and their tests to util.
These are dependencies of the upcoming MinidumpStringWriter.

R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/430003003
2014-08-01 14:39:55 -04:00