| Age | Commit message (Collapse) | Author |
|---|
|
BUG: 148941208
test: TH
Change-Id: I1134f1e9e968b9273748e2483bea8d25e5c9e994
|
|
|
|
Fixes:
system/core/base/include/android-base/result.h:
133:94: warning: potentially unintended semicolon [bugprone-suspicious-semicolon]
Bernie says:
it probably means that there's a parser bug with "if constexpr"
maybe, at static analysis pass, the "if constexpr" was evaluated to false,
and the compiler removed the "then" block from the AST...
... and then it thought you had written it that way :-)
https://reviews.llvm.org/D46027
Test: builds
Bug: 153035880
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I25df8eeca4ec06b3180c1cd21b554fc583c5581a
|
|
Fixes:
system/core/base/include/android-base/expected.h:
186:13: warning: constructor accepting a forwarding reference can hide the copy and move constructors [bugprone-forwarding-reference-overload]
195:22: warning: constructor accepting a forwarding reference can hide the copy and move constructors [bugprone-forwarding-reference-overload]
611:13: warning: constructor accepting a forwarding reference can hide the copy and move constructors [bugprone-forwarding-reference-overload]
To quote Tom Cherry:
I'm a bit confused at what's happening there.
I think it's a bug in the linter itself.
The general solution to that problem is a heavy dose of std::enable_if<>
to hide that constructor when the 'U' parameter is the same class,
but those constructors do have the necessarily std::enable_if<> lines.
I think the problem is that the linter doesn't check that the macro
_ENABLE_IF() expands into std::enable_if<>. Let me try explicitly
putting the std::enable_if<> instead of the macro and check if it
goes away.
I expanded the macro but the linter doesn't still doesn't accept
the format of `std::enable_if_t<(condition_here)>* = nullptr`.
It does accept `typename Enable = std::enable_if_t<(condition_here), void>`,
which is the syntax used on their example here:
https://clang.llvm.org/extra/clang-tidy/checks/bugprone-forwarding-reference-overload.html.
That latter syntax doesn't work for us.
See the Notes section on
https://en.cppreference.com/w/cpp/types/enable_if
as a reference for why what we're doing is correct.
Test: builds
Bug: 153035880
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I493ff19208cc104f5f176a36ec23fbcb914388f7
|
|
Previously, we would split messages by line and call the logger
function for each line. We would hold a lock during this, to ensure
that multiple threads would not interleave their messages.
There are a few problems with this approach:
1) Using a lock is not efficient and is not fork safe
2) With APEX, there is one lock per instance of libbase, so we must
move the lock to a location where all instances can access it, or
perform the line splitting in a way that does not require the lock.
To solve these issues, we reimagine line splitting.
1) We move the lock out of the LogMessage::~LogMessage() and make it
the logger's responsibility to split lines, giving the logger the
option to lock or not.
2) We do not need any locks at all for StderrLogger.
Instead, we generate a single string that contains all of the lines
with their appropriate log header. A single write() call is used
to output this at once.
3) Logd handles log messages with newlines correctly, however it only
accepts up to a maximum size of log message. Therefore we
separate the incoming log message into chunks, delimited by new
lines, up to that maximum size, and send each of those to logd.
Note that this is the strategy used in
android.util.Log.printlns().
This should solve a majority of use cases, since the maximum size
that logd accepts is nearly 4K, while remaining lock free.
If interleaving messages absolutely must be avoided, a lock can
still be used given 1) above.
Bug: 65062446
Bug: 153824050
Test: logging, particularly multi-line stack traces, show correctly
Test: existing and new unit tests
Change-Id: Id0cb5669bee7f912da1e17f7010f0ee4c93be1e3
|
|
Fixes:
system/core/base/include/android-base/expected.h:606:39: warning: repeated branch in conditional chain [bugprone-branch-clone]
if (x.has_value() != y.has_value()) {
^
system/core/base/include/android-base/expected.h:608:4: note: end of the original
} else if (!x.has_value()) {
^
system/core/base/include/android-base/expected.h:610:10: note: clone 1 starts here
} else {
^
Test: builds
Bug: 153035880
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Ie67a8bb1bf622319adea15466c42077e0e9b1a18
|
|
Appease clang-tidy by marking reset() as a method that reinitializes
after moving out of a unique_fd.
Unfortunately, there isn't an attribute that let us mark the type as
being safe to use after move in general, which means that moving out of
a unique_fd and then calling get() on it will still be frowned upon.
clang-tidy has a hard-coded list of standard container types that
are safe to use after move, but doesn't provide a way to mark custom
types as satisfying this condition.
Bug: http://b/150959261
Test: reverted the change to unique_fd.h and the test failed
Change-Id: Ide73d7caa4cd2b192018f111059d696dca4de987
|
|
Copy bionic's CachedProperty with some minor API tweaks, to allow for
efficient querying of properties that rarely change.
Bug: http://b/141959374
Test: treehugger
Change-Id: I4dfc3f527d30262b35e871d256cec69e69f2e1d7
|
|
Test: treehugger
Change-Id: I68fcd5da304d04ff4da3c3f3712fb79ce6b5791e
Merged-In: I68fcd5da304d04ff4da3c3f3712fb79ce6b5791e
|
|
This is no longer needed
Test: build
Change-Id: Ied9b26ff517906d662f90a31c98829843c1e9a9f
|
|
|
|
These macros are meant to be used in tests:
Result<File> maybe_a_file = OpenFile(...);
EXPECT_OK(maybe_a_file);
On failure, the error is printed.
There's no equivalent EXPECT_NOT_OK() because it is a testing anti-pattern
which causes tests to pass even when the error changed as a result of a
regression. Use EPECT_EQ(result, Error(...)) instead.
Test: cd system/core && atest
Test: m
Change-Id: Ie26f90d3c62620e7b1f10013829ba43ef5364fe1
|
|
This is an alias for has_value() which is meant to be more concise, yet
more explicit than the conversions to bool.
Leave operator bool() in place for now: due to the large number of
users, removal of operator bool() needs to be broken up into incremental
stages.
Test: cd system/core && atest
Test: m
Change-Id: Ib1eb00f47d3c0f2229bb176b62687463b834f280
|
|
fmtlib provides compile time checking of format strings that we're not
currently using. This change makes Errorf() and ErrnoErrorf() into
macros such that we can take advantage of this capability.
Test: build successfully normally
Test: fail the build if using an invalid format string
Change-Id: Icb8ba8cb973bbd1fa4755a62e7598bdbb0113757
|
|
These operators were included because they're present in the draft
standard proposal of std::expected, but they were deemed to lead to
bugs, particularly when T is implicitly convertible to bool.
Change-Id: Ib149decf1f230198f358dc1ae0eaed71961363f6
Test: m
|
|
Some recent changes can have these logging functions potentially set
errno. This change places android::base::ErrnoRestorer at the entry
point of the public functions where we want to guarantee errno is
restored to ensure this will not happen again.
Test: build
Change-Id: Iab4170ab16b9c7301474a509ee42d38b370b91a4
|
|
Test: builds without warnings
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Iac7c63816dd7bf2536c0d465ef1e845330be505b
|
|
Bug: 119867234
Test: log tags look right, libbase/liblog unit tests
Change-Id: I3670c3fdce3d0238a23a53bba2877ffed1291f9c
|
|
|
|
See the previous commit moving SetLogger and SetAborter to liblog for
motivation.
This creates more harmony between the two mechanisms in libbase and
liblog for checking loggability.
Currently:
1) libbase filters all messages based on its minimum log priority. For
example, if minimum log priority in libbase remained at its
default, but a tag was specifically opted into DEBUG logs via
log.tag.<tag>, libbase would not print this log.
2) liblog ignores libbase's minimum log priority. For example if a
process called SetMinimumLogPriority(WARNING) but used a library
that logged via liblog's ALOGI macro, that log would still be
printed even though the process intends on filtering out those INFO
messages.
With this change:
1) If both a minimum log priority and a priority through log.tag.<tag>
are set, then the lower of the two values is used.
2) If only one or the other is set, then that value is used. This
fixes the two issues described above.
3) If neither of these values are set, then the default of using INFO
is unchanged.
Bug: 116329414
Bug: 119867234
Test: libbase and liblog minimum log priority tests
Change-Id: Icb49b30b9d93bf797470e23730ae9e537931bb6c
|
|
libbase is copied into each APEX module which requires it, meaning
that there may be multiple instances of libbase running within a
single process with their own copy of libbase's globals. This means
that SetLogger() and SetAborter() will only impact logs from the
instance of libbase that calls it. This change moves this state to
liblog, since it will only ever have one instance in a single
process.
One major side-effect here is that now both ALOGE style and LOG(...)
style logs will be handled through the same logger function. For
example, a logger specified through libbase's SetLogger() will now see
logs sent to liblog through ALOGE(). This is intended behavior.
A second side-effect is that libbase's stderr logger is used for all
host logging now. It's simply a better logging default than the
fake_log_device logger in liblog currently and makes ALOGE and
LOG(...) logs on host follow the same format.
Bug: 119867234
Test: libbase and liblog unit tests; logging works
Change-Id: Ib52cbfb4e43749e50910ed19a993dffae19ace86
|
|
This is for projects built with GCC that import parts of Android that,
despite not having Android-specific dependencies, still end up
depending on logging.h.
Also removes outdated notes.
Change-Id: I5a47b302bcaeeb935592d8fc7ad2fe5068d226c3
|
|
|
|
|
|
With C++20, any use of the existing overloads with two unique_fd
arguments is likely to become ambiguous:
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20200113/301880.html
Newer versions of clang complain about this ambiguity. Fix the error by
adding overloads that take two unique_fds.
Bug: 145916209
Change-Id: I18a292827d8841b6d24f948682123ab54dc7aaca
|
|
There were only two users of these and both of them were better off
setting up a default logger.
These macros are not particularly useful as it's not useful for a
single program to write to both the MAIN and SYSTEM logs. In fact,
the opposite of these macros would be more beneficial: having more
programs write to only the MAIN or only the SYSTEM buffer, so getting
rid of these macros removes a temptation for bad behavior.
Users that absolutely need to do this behavior can still use the
liblog macros or functions, but that should be an extreme edge case,
such as the few programs that write to the CRASH buffer and does not
need to exist in libbase.
Bug: 119867234
Test: build
Change-Id: I23369c3b48ed636b617220cab47f77fdd5559763
|
|
liblog will soon be required for all of libbase's logging. This
change proactively requires liblog in all configurations instead of
just Android.
Bug: 119867234
Test: build
Change-Id: I696162fbebc78d4ef23c6032412101ac51d397a4
|
|
d77c99ebc3d896501531e9522b690c4a0e971aff changed MappedFile to return a
bogus zero-length mapping on failure rather than nullptr. None of the
calling code was changed, though, and it seems like doing so would be a
bad idea. Revert that part of the change.
Add missing tests, and tidy up some of the logging. Also remove
single-use or obfuscatory constants from the tests.
The new "empty.zip" was created by using zip(1) to create a zip file
with one entry, then using `zip -d` to remove it.
The new "zero-size-cd.zip" was created by using zip(1) to create a zip
file containing a single empty file, and then hex editing the two byte
"size of the central directory" field in the "end of central directory
record" structure at the end of the file. (This is equivalent to, but
much smaller than, the example zip file provided by the bug reporter.)
Bug: http://b/145925341
Test: treehugger
Change-Id: Iff64673bce7dae886ccbc9dd6c2bbe18de19f9d2
|
|
Previously, in the regex test helpers, we would evaluate the haystack
expression again to generate the error message, which leads to
nonsensical errors if the expression returns a different value on the
second call (e.g. functions like dlerror which return null on subsequent
calls).
Test: bionic-unit-tests with a failure
Test: treehugger
Change-Id: I2126cefeb45e26638194af8a82d0f2a9d7196edf
|
|
|
|
Pillage from Chromium a wrapper type that skips destruction of its
wrapped type, to avoid problems with premature destruction of
variables with static lifetime.
Test: libbase_test on host
Change-Id: I7d4541f7b59f467b232d5c4f8250dc1ea45e28fa
|
|
Also change the properties implementation to call the new API. We use
this ParseBool API in the new SystemProperties implementation, with
which we want the libbase property API to be consistent.
Test: included
Change-Id: I89cb3eb4e1203a6bb0da41914dad720e44c00303
|
|
Needed for cli-test.
Test: treehugger
Change-Id: Ib1fd01ef7f3e54e5778cc548dd789b5fcfcb7bd9
|
|
This was added with the intention of using it in adb, but then the
change that would have used it
(https://android-review.googlesource.com/c/platform/system/core/+/273824)
was abandoned.
Remove the corresponding (never used) implementation.
Bug: http://b/31468413
Test: treehugger
Change-Id: I42322d079c175b7c6fbd12940e5bc022bd9ebd1c
|
|
<winsock2.h> has defined htonl(), ntohl(), htons() and ntohs() with
different return type that cannot be replaced by those macro definitions
in <android-base/endian.h>.
Includes <winsock2.h> first to prevent them from being replaced.
Then defines the macro later so we don't need to call into DLL
when using those functions.
Bug: 139639521
Test: m libbase_test && wine out/host/windows-x86/nativetest64/libbase_test/libbase_test64.exe
Test: m checkbuild
Change-Id: I672f23a31c6800df10e04d36695d446bca4c91e9
|
|
See https://gcc.godbolt.org/z/hGUZIk
Test: inspection
Bug: 140506797
Change-Id: I6fe87b16e18c93b7fc3ca28edbbc68c245e7e5e5
|
|
Upstream merged time.h into chrono.h
Test: build
Change-Id: Iadbdfa4902ddd2dd38ee1dba51ef59def54130e4
|
|
|
|
adb uses its own implementation of Windows HANDLE->int mapping,
and it doesn't play well with _get_osfhandle() function used in
MappedFile::FromFd().
This CL adds another function that accepts raw handle, so adb
can pass it directly
+ make constant functions 'const'
+ make the MappedFile movable, as nothing prevents it from being
one
Test: libbase_test
Change-Id: Ifde4a4094b910e9c7b431126ecf3fef5aa3bb4a6
|
|
__BIONIC__ is defined in sys/cdefs.h, __GLIBC__ is defined in
features.h (which is included from sys/cdefs.h). If sys/cdefs.h
was not included before android-base/endian.h it was always falling
back to the Windows definitions.
mingw defines LITTLE_ENDIAN, BIG_ENDIAN and BYTE_ORDER in
sys/params.h, use those definitions to avoid conflicts.
glibc uses different names for letoh*, add compatibily #defines.
Test: m checkbuild
Change-Id: I0709a964cc8f20dd9fa4f03064cc67d97ae6c525
|
|
Turns out that although there's no <endian.h> or <sys/endian.h>, there
are <machine/endian.h> and <sys/_endian.h>, and they're included by
other system headers such as <dirent.h>.
Reuse the contents of <sys/_endian.h> here for better interop.
Bug: http://b/139203733
Test: treehugger
Change-Id: Ic0e9bfa1a5b56d05e9e542839d237b6ceae4aa8c
|
|
|
|
While I'm here, I'll fix how unique_fd disallows copy and assignment
constructors (detele instead of marking them private).
Bug: 135918744
Test: WiP change in master
Change-Id: Idefcc685943326c511f59d18790c1c4fa2e04989
|
|
|
|
bpfloader uses both clang-tidy and this header, so the below lint
warnings are generated:
/work/aosp/system/core/base/include/android-base/unique_fd.h:261:18:
warning: single-argument constructors must be marked explicit to
avoid unintentional implicit conversions
[google-explicit-constructor]
/* implicit */ borrowed_fd(int fd) : fd_(fd) {}
^
explicit
/work/aosp/system/core/base/include/android-base/unique_fd.h:263:18:
warning: single-argument constructors must be marked explicit to
avoid unintentional implicit conversions
[google-explicit-constructor]
/* implicit */ borrowed_fd(const unique_fd_impl<T>& ufd) : fd_(ufd.get()) {}
^
explicit
Add NOLINT to quiet them.
Test: build without this lint warning
Change-Id: I5241938c33070b0fa39888289b8ca67d6d94ac73
|
|
These move and assignment operations are conditionally noexcept, so
add that to their definitions. This is a performance issue as well as
correctness, since std containers will copy instead of move during
resize if these operations are not noexcept.
Test: build
Change-Id: I148f7eb3489e7f1dd68cc0fb0e555b56470e42da
|
|
|
|
clang-tidy flagged the anonymous namespace in the header as a warning,
but in any case, it'll be cleaner to implement this in terms of the
existing type traits.
Test: build
Change-Id: I189986d2a855c028e28dd9d62ab9da012feddc9b
|
|
android-base:
* Add NOLINT for expanding namespace std for std::string* ostream
overload
libdm:
* Fix missing parentesis around macro parameters
init:
* Fix missing CLOEXEC usage and add NOLINT for the intended
usages.
* Fix missing parentesis around macro parameters
* Fix erase() / remove_if() idiom
* Correctly specific unsigned char when intended
* 'namespace flags' should be signed, since 'flags' it signed for
clone()
* Add clear to property restore vector<string> to empty after move
* Explicit comparison against 0 for strcmp
Test: build
Change-Id: I8c31dafda2c43ebc5aa50124cbbd6e23ed2c4101
|
|
We loop over /proc to iterate through pids in a lot of code, so let's
consolidate this into a single function in libbase.
Test: new unit test
Change-Id: I908fab90b603546d0e3e8b8acdc8dadfc3552d62
|
