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>
Although GCC will silently accept larger alignments with
__attribute__((aligned())), it warn on alignas() with an alignment
larger than the target’s supported maximum. 8c35d92ae403 switched to
alignas() where possible.
The maxima are at least 128 on x86, x86_64, and arm64, and 64 on arm, in
the common configurations, but may be even larger with certain features
such as AVX enabled. These are ultimately derived from BIGGEST_ALIGNMENT
in gcc/config/*/*.h.
One alignment request in a test specified 1024 as a big alignment
constraint, solely as a test that alignment worked correctly. For this,
it’s perfectly reasonable to limit the alignment request to what GCC
supports on the most constrained target we’ll encounter.
Test: crashapd_util_test AlignedAllocator.AlignedVector
Change-Id: I42af443f437e01228934ab34dc04983742f0ab3f
Reviewed-on: https://chromium-review.googlesource.com/550236
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
This was missed in Crashpad 8c35d92ae403. It syncs with Chromium
16289b3ef759.
Change-Id: I7e92e71fc940e25e751e7487d100b5684bdbf667
Reviewed-on: https://chromium-review.googlesource.com/535577
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Use the standard alignas instead of ALIGNAS in cases where this is
possible too. It’s not currently possible where ALIGNAS may be mixed
with other attributes, although the not-landed
https://codereview.chromium.org/2670873002/ suggests that where ALIGNAS
is mixed with __attribute__((packed)), it’s viable to write “struct
alignas(4) S { /* … */ } __attribute__((packed));”.
This includes an update of mini_chromium to
723e840a2f100a525f7feaad2e93df31d701780a, picking up:
723e840a2f10 Remove ALIGNOF
This tracks upstream https://codereview.chromium.org/2932053002/.
Change-Id: I7ddaf829020ef3be0512f803cecbb7c543294f07
Reviewed-on: https://chromium-review.googlesource.com/533356
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
76a67a37b1d0 adds crashpad_handler’s --monitor-self argument, which
results in a second crashpad_handler instance running out of the same
database as the initial crashpad_handler instance that it monitors. The
two handlers start at nearly the same time, and will initially be on
precisely the same schedule for periodic tasks such as scanning for new
reports to upload and pruning the database. This is an unnecessary
duplication of effort.
This adds a new --no-periodic-tasks argument to crashpad_handler. When
the first instance of crashpad_handler starts a second to monitor it, it
will use this argument, which prevents the second instance from
performing these tasks.
When --no-periodic-tasks is in effect, crashpad_handler will still be
able to upload crash reports that it knows about by virtue of having
written them itself, but it will not scan the database for other pending
reports to upload.
Bug: crashpad:143
Test: crashpad_util_test ThreadSafeVector.ThreadSafeVector
Change-Id: I7b249dd7b6d5782448d8071855818f986b98ab5a
Reviewed-on: https://chromium-review.googlesource.com/473827
Reviewed-by: Robert Sesek <rsesek@chromium.org>
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-comparison77d6b17338https://github.com/google/googletest/pull/713
Change-Id: I978fef7c94183b8b1ef63f12f5ab4d6693626be3
Reviewed-on: https://chromium-review.googlesource.com/466727
Reviewed-by: Scott Graham <scottmg@chromium.org>
Remove stl_util from Crashpad. This also updates mini_chromium to
4f3cfc8e7c2b7d77f94f41a32c3ec84a6920f05d to remove stl_util from there
as well.
4f3cfc8e7c2b Remove stl_util from mini_chromium
BUG=chromium:555865
Change-Id: I8ecb1639a258dd233d524834ed205a4fcc641bac
Reviewed-on: https://chromium-review.googlesource.com/438865
Reviewed-by: Scott Graham <scottmg@chromium.org>
While building crashpad_database_util.cc:
…\crashpad\tools\crashpad_database_util.cc(150) : error C3861: 'gettimeofday': identifier not found
util/win/time.h has its own GetTimeOfDay() to provide this missing
function on Windows. I don’t know why it’s not in compat. Even so, it
doesn’t return a value, so it’d be unsuitable for use in the PCHECK().
Go back to time() with an errno test.
While building string_number_conversion_test.cc:
…\crashpad\util\stdlib\string_number_conversion_test.cc(242) : error C2220: warning treated as error - no 'object' file generated
…\crashpad\util\stdlib\string_number_conversion_test.cc(242) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
…\crashpad\util\stdlib\string_number_conversion_test.cc(243) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
…\crashpad\util\stdlib\string_number_conversion_test.cc(244) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
Use INT64_C(), and remove a duplicate test case.
Change-Id: I308db9856e492604c7462238cb8b7b66731f0cfe
Reviewed-on: https://chromium-review.googlesource.com/411331
Reviewed-by: Robert Sesek <rsesek@chromium.org>
The database settings object’s last_upload_attempt_time (time_t) field
is switched from uint64_t to int64_t, for better compatibility with
time_t, which is normally a signed type. This change should be
transparent, as there should be no valid high-bit-set 64-bit timestamps
in this field in the wild.
A number of improvements are made to crashpad_database_util’s time
handling. Errors are checked during time conversion.
--set-last-upload-attempt-time=now is a new supported (and documented)
option.
A StringToNumber() overload for int64_t, along with a test, is added to
aid in crashpad_database_util’s time conversions from numeric strings. A
test is also added for the previously-untested uint64_t implementation.
TEST=crashpad_util_test StringNumberConversion.*
Change-Id: I089c4bf7b95f5df0982bdbb3c27b4f6a89db966e
Reviewed-on: https://chromium-review.googlesource.com/410068
Reviewed-by: Robert Sesek <rsesek@chromium.org>
This makes Doxygen’s output more actionable by setting QUIET = YES to
suppress verbose progress spew, and WARN_IF_UNDOCUMENTED = NO to prevent
warnings for undocumented classes and members from being generated. The
latter is too noisy, producing 721 warnings in the current codebase.
The remaining warnings produced by Doxygen were useful and actionable.
They fell into two categories: abuses of Doxygen’s markup syntax, and
missing (or misspelled) parameter documentation. In a small number of
cases, pass-through parameters had intentionally been left undocumented.
In these cases, they are now given blank \param descriptions. This is
not optimal, but there doesn’t appear to be any other way to tell
Doxygen to allow a single parameter to be undocumented.
Some tricky Doxygen errors were resolved by asking it to not enter
directiores that we do not provide documentation in (such as the
“on-platform” compat directories, compat/mac and compat/win, as well as
compat/non_cxx11_lib) while allowing it to enter the
“off-platform” directories that we do document (compat/non_mac and
compat/non_win).
A Doxygen run (doc/support/generate_doxygen.sh) now produces no output
at all. It would produce warnings if any were triggered.
Not directly related, but still relevant to documentation,
doc/support/generate.sh is updated to remove temporary removals of
now-extinct files and directories. doc/appengine/README is updated so
that a consistent path to “goapp” is used throughout the file.
Change-Id: I300730c04de4d3340551ea3086ca70cc5ff862d1
Reviewed-on: https://chromium-review.googlesource.com/408812
Reviewed-by: Robert Sesek <rsesek@chromium.org>
StringToUnsignedInt[64]Traits::Convert() was returning in its failure
(negative input) case without touching *end. Its caller relies on *end
to detect failure.
Change-Id: I636f95471cd499434743e73f0e5e0b60c0871795
Reviewed-on: https://chromium-review.googlesource.com/405468
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Previously, StartHandler() launched the handler process, then connected
over a pipe to register for crash handling. Instead, the initial client
can create and inherit handles to the handler and pass those handle
values and other data (addresses, etc.) on the command line.
This should improve startup time as there's no need to synchronize with
the process at startup, and allows avoiding a call to CreateProcess()
directly in StartHandler(), which is important for registration for
crash reporting from DllMain().
Incidentally adds new utility functions for string/number conversion and
string splitting.
Note: API change; UseHandler() is removed for all platforms.
BUG=chromium:567850,chromium:656800
Change-Id: I1602724183cb107f805f109674c53e95841b24fd
Reviewed-on: https://chromium-review.googlesource.com/400015
Reviewed-by: Mark Mentovai <mark@chromium.org>
The utilities in base/stl_util.h have been moved from the global
into the base namespace. This patch updates the call sites accordingly.
No functional changes.
Change-Id: I059d5d6299f947b1135672da170427d23ac4775e
Reviewed-on: https://chromium-review.googlesource.com/368640
Reviewed-by: Mark Mentovai <mark@chromium.org>
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 .
This more-natural spelling doesn’t require Crashpad developers to have
to remember anything special when writing code in Crashpad. It’s easier
to grep for and it’s easier to remove the “compat” part when pre-C++11
libraries are no longer relevant.
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1513573005 .
MEMORY_BASIC_INFORMATION64 specifies an alignment of 16, but the
standard allocator used by containers doesn't honor this. Although 16
is the default alignment size used on Windows for x86_64, it's not for
32-bit x86. clang assumed that the alignment of the structure was as
declared, and used an SSE load sequence that required this alignment.
AlignedAllocator is a replacement for std::allocator that allows the
alignment to be specified. AlignedVector is an std::vector<> that uses
AlignedAllocator instead of std::allocator.
BUG=chromium:564691
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1498133002 .
The warnings are emitted when a translation unit attempts to reference
a function whose availability is newer than the deployment target.
BUG=471823
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1109273002
Patch from Erik Chen <erikchen@chromium.org>.
Add MapInsertOrReplace<>() to insert a key-value pair into a map if the
key is not already present, or replace the existing value for key if the
key is present. The original value can optionally be returned to the
caller in this case.
Map insertions now use either MapInsertOrReplace<>() or
std::map<>::insert() directly.
Use MapInsertOrReplace<>() when the map should be updated to contain a
mapping from a key to a value regardless of whether the key is already
present.
Use std::map<>::insert() to insert a mapping from a key to a value
without replacing any existing mapping from a key, if present. If it is
important to know whether an existing mapping from a key was present,
use the returned std::pair<>.second. If it is important to know the
existing value, use the returned std::pair<>.first->second.
This change has a slight positive impact on performance.
TEST=crashpad_util_test MapInsert.MapInsertOrReplace and others
BUG=
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1044273002
CXX_LIBRARY_VERSION is irrelevant, because the only C++11 library
feature of any concern is whether numeric_limits’ min() and max() are
declared constexpr.
Crashpad is C++11-only as far as the language is concerned, and the
comment doesn’t need to call it out explicitly because static_assert()
is always available.
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/809833002
Unfortunately VS2013's support of C++11 is partial. It supports the
extended union definition, but does not fully support constexpr.
So, update some locations where CXX_LIBRARY_VERSION is used where
toolchain support is lacking. It works correctly in the locations
where std::is_standard_layout is used.
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/803273002
This only came up in one location, PointerVector.
A template alias is superior to inheritance, which doesn’t provide full
type equivalence and doesn’t automatically inherit non-default
constructors.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/683753005
This includes the StringToNumber() function, both int and unsigned int variants.
Similar functionality is available in base, but it is unsuitable for
applications where a number’s base may be determined based on an "0x" or
"0X" prefix (hexadecimal) or an "0" prefix (octal).
TEST=util_test StringNumberConversion.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/557033002
