Remove the transport available and open functions since the writers
are able to manage their own state. Remove the initialization dance
with write_to_log, since it is unneeded once this is removed as well.
Remove the global lock around the close() functions as correct locking
has been added to the writers in a previous change.
Test: logging works, liblog-unit-tests
Change-Id: If7fa11e773763d0b5fcb2e696ad1c88ff4a4cfdf
The current system of using atomics isn't thread safe and may result
in doubly closing FDs or closing actively used FDs. The safest way to
do this is to use a rwlock, which should not have a much higher
overhead than the atomics do, as a vast majority of the time, there
will not be writers.
This moves us further away from using the transport interface, which
will be removed. Each writer should be self contained, without a
separate open or available function.
Also, keep the pmsg fd open if it is opened by
__android_log_pmsg_file_write(). This fd was closed due to issues
with zygote, but it looks like it is only called by recovery now, so
there is no reason to close this fd at the end of that function.
Test: logging works, liblog-unit-tests
Change-Id: I345c9a5d18c55b11a280c8362df854784abf46fd
This code was introduced to help performance by skipping sending these
messages to logd when logd would later drop them. However it has
multiple flaws:
1) Event logs aren't super common and non-loggable event logs are even
less common, so it would be a trivial benefit if any.
2) This code is not particularly safe as written, which is even
acknowledged in the comments.
3) This forces processes that write event logs to allocate a rather
sizable amount of memory.
Therefore, it's better to simply remove this and let logd drop these
messages when it receives them.
Bug: 139705697
Test: logging works, liblog-unit-tests
Change-Id: Ide01574112e173d4922137b3d3868cf8c2c09086
This still fakes the long removed /dev/log devices, whereas it only
needs to print to stderr, so simplify that code.
Use std::mutex now that it is C++ to easy portability concerns.
Use the proper liblog headers for formatting information instead of
hardcoding a copy.
Test: liblog-host unit test
Change-Id: I310a6e7ad939960300eafa729cbfc535c5ced445
The current version requires callers to supply a string with 32 extra
bytes for liblog to internally prepend "setPruneList ", and to have
enough space to parse logd's return string. That is an unacceptable
requirement on callers.
This change removes that requirement by having liblog allocate the
needed std::string in any case.
It also stops writing back the 'success' or 'Invalid' string to the
caller's buffer, since that is redundant as well.
Test: changing prune settings works.
Change-Id: Ic0f03a229f0b9a77d03adcb91288370c3bd42903
We don't need to be so strict about this comparison. It's possible
that logd will extend the message that it passes to readers in the
future, and since we have a hdr_size parameter it can do so in a
backwards compatible way, as long as we loosen this restriction.
This keeps a sane upper bound that the hdr_size cannot be larger than
the log message itself.
Test: logcat, liblog-unit-tests
Change-Id: I8a6bea2a2d6e3315d998c51c1029e466ff06b45f
This is unneeded, since we're already checking the length returned by
recv() and log_msg that is read for validity.
It costs ~4% of CPU with `logcat -s` and ~2% of CPU when running
simpleperf for 1 second on walleye on master.
Bug: 144311420
Test: logcat works, simpleperf doesn't show memset() costing as much.
Change-Id: I986e7e96518774034340f1b1201a2071a904e3bb
This is simplified down to the point there are only two branches that
need to be made, so remove the rest of the transport structs and
simply branch where needed.
Test: liblog-unit-tests
Change-Id: Ic82e7e70eb7b4e40b381a4d8066629c5b7d4f827
Shifting a signed 32-bit value by 31 bits is implementation-
defined behavior. So we change to an unsigned value for our
shift by 31 bits, and go ahead and change the others to
unsigned for consistency.
Test: TreeHugger
Change-Id: Ib98f9b1e468d28dafd09e86273bf76beb1ea1fa5
There are a set of functions, such as android_logger_get_log_size()
and android_logger_get_prune_list() that talk to the logd command
socket to perform their activities. There's a transport abstraction
layer that handles these symbols to optionally route them to other
transports, originally designed for pstore or local logger; however
these functions fundamentally only make sense for logd.
Ideally, these functions would be removed and new functions would be
added that do not depend on struct logger_list or struct logger and
more clearly indicate that they only work with logd. For example:
android_logger_get_size(struct logger*) could be
logd_get_buffer_size(log_id_t log_id). We would remove the need to
'open' the struct logger and make it clear that it only operates on
logd.
Since liblog is an llndk library however, we cannot change or remove
these symbols. Since these symbols are not frequently used, it seems
acceptable to keep them as is and not introduce improved versions.
We, however, do want to simplify the code that handles them and this
change removes the transport abstraction layer that handles them.
They retain the behavior that unless the struct logger_list was opened
for logd, that the functions return -EINVAL.
The one exception to this is android_logger_clear(). If the struct
logger provided to this function was opened from a struct logger_list
that used pstore for its mode argument, this function will clear the
entire pstore log. This function does not respect the 'logId'
parameter of the struct logger, since that would not be possible.
This change removes this android_logger_clear() behavior and makes it
strictly for logd, for symmetry with the rest of the functions and due
to the lack of clarity regarding the 'logId' parameter of its input.
The only caller of this function, logcat, will clear pstore directly.
struct logger was built to encapsulate the information needed to
connect to a logger device from the old kernel logger. Now that we
only support reading from pstore and from logd, there is much less
information needed to be captured. Specifically, we only need to know
the log_id and whether or not it was opened as part of a pstore or
logd 'list'.
Test: liblog-unit-test
Test: logcat -c/-g/-G/-p/-P/-S work
Test: logcat -c works with -L
Test: logcat -g/-G/-p/-P/-S continue to fail with -L
Change-Id: I2c549b6f8539de94510e223949ab209ecc40e2d0
We used to do this, but it got lost while refactoring this code.
Bug: 144311420
Test: we see "unexpected EOF!" instead of "unexpected length" from logcat
Change-Id: I7858d0a774a9eac63e5547ee67e85ef8fb0c682d
There's a lot of unnecessary boilerplate around these opaque types,
and this change simplifies it.
Test: liblog-unit-tests
Change-Id: I0c4e133037fd5f04157ac22175181a6a496e18c4
The APIs that are tagged with # vndk are actually for LLNDK libraries.
Although LLNDK is part of VNDK, calling those APIs 'vndk' has given
users a wrong perception that the APIs don't need to be kept stable
because that's the norm for most of the VNDK libraries that are not
LLNDK.
In order to eliminate the misunderstanding, rename the tag to 'llndk' so
that people introducing new such API will realize what they are signing
themselves up for.
Bug: 143765505
Test: m
Merged-In: Iae2acdf1ff4097a64a5c6280797c66abb1d5a5e6
(cherry picked from commit 0e957b82c8)
Change-Id: Iae2acdf1ff4097a64a5c6280797c66abb1d5a5e6
This protocol documentation is spread out among various functions
where it is implemented. This change makes a single
README.protocol.md file with a high level overview, referencing the
struct names where useful.
Test: n/a
Change-Id: I83c9f484352b489b4a20cce241d92413f780f9ec
logger_entry and logger_entry_v2 were used for the kernel logger,
which we have long since deprecated. logger_entry_v3 is the same as
logger_entry_v4 without a uid field, so it is trivially removable,
especially since we're now always providing uids in log messages.
liblog and logd already get updated in sync with each other, so we
have no reason for backwards compatibility with their format.
Test: build, unit tests
Change-Id: I27c90609f28c8d826e5614fdb3fe59bde22b5042
I'm about to make a bunch of changes that are safe for backwards
compatibility, but would otherwise flag the checker, so it's time to
temporarily disable it.
Test: CLs pass despite making header changes
Change-Id: Ibc2d4ae51fb8e6125b9117ccd92bf821db945e67
This reverts commit 5f8162b086.
Reason for revert: Turns out they're being used.
Merged-In: Iad9010190c7a4140b69dc553df5debdd88dcf81a
Change-Id: Iad9010190c7a4140b69dc553df5debdd88dcf81a
These functions and headers were all mistakenly added to the vndk.
They should not be used by vendors.
Test: these symbols do not appear in vendor libraries
Merged-In: I03919b437c2d9f0e573b7a6b40249ed12fe874b9
Change-Id: I03919b437c2d9f0e573b7a6b40249ed12fe874b9
1) We don't need two copies of log_id_t
2) We don't need misleading sizeof_log_id_t or typeof_log_id_t macros
3) logd should use android_log_header_t explicitly for its recv buffer
size
4) Following on from b/129272512, we're settling that returning
LOG_ID_MAX is an acceptable return value from
android_name_to_log_id().
Bug: 129272512
Test: build, liblog, logcat unit tests
Change-Id: I67fb964a4a0ae9cb6e1514ca110e47e00dfcfa9a
There's no point in client side security checks in this library. If a
process has access to these files, then they'll be able to do any of
these operations themselves.
Test: liblog unit tests
Change-Id: I75d4e1509eb8ff0ac4579f820a8968f4f5ad4e06
Enable more of the disabled tests. These should not be flaky and have
value.
The dual_reader test is rewritten in the style of RunLogTests() as
well.
Test: 100 iterations of these tests on CF without failure
Change-Id: I15de9e21b066aa22635cc0bd71b51e2648198823
The log_time struct satisfies all of the requirements for an
implicitly created copy constructor to be present, so not defining one
here does not have any real effect.
We don't want to delete the copy constructor for the rationale given
either; modern C++ favors passing small types by value instead of by
reference as the compiler has more opportunity for optimization in
that case. That's especially true here, where the size of this struct
is the size of a pointer on 64 bit systems.
Test: the copy constructor exists for log_time
Change-Id: Id314ca7729f4b1ca02adb6c7f0ae759b22be2a5c
Continuing the speed up / clean up from the last change.
Enable a subtest that was previously disabled as well. It passes 100s
of cycles now on CF.
Test: liblog-unit-tests
Change-Id: Ifff6f400c3736a1a857a3fdaf22d7ef1794abf9b
A lot of liblog tests follow this pattern:
1) Write a log message
2) Sleep ~1 second
3) Use the non_blocking log reader to dump all log messages
4) Test those log messages
This causes running back to back tests to be very slow and still
allows for some amount of flakiness if the system is very loaded.
This change replaces that pattern with the following:
1) Write a log message
2) Set an alarm for 2 seconds as a test timeout
3) Read logs with the blocking reader until finding the expected log
messages
4) Test those log messages
5) Use the non_blocking reader to dump all log messages
6) Re-test those log messages, to ensure no duplicates, etc, which
isn't done in step 3).
Despite dumping the logs twice, the tests are orders of magnitude
faster in the good case, and should be less prone to flakes.
Test: liblog-unit-tests
Change-Id: Iedf473316576b8007746fe3560815bde1813787a
Per jmgao@, we still need to worry about unaligned integer accesses.
In any case, we already have the packed structs for all of the
unaligned data that we're reading, so this change favors using those
packed structs.
Bug: 142256213
Test: x86,arm32,arm64 liblog-unit-tests
Change-Id: I21fc629eac49895d03b5b31daa4cc494b0c4c230
There is an alarm() call that provides a 30 second timeout in case
logd is unavailable. The main reason for this is in the case that logd
is crashing, debuggerd is ptrace'ing logd, and tombstoned is
attempting to dump log messages. In this case, with no other checks
and without this alarm, tombstoned will deadlock when dumping log
messages.
However, tombstoned already has two mechanisms to prevent the above
situation from happening:
1) It checks that the thread name that is is dumping is either logd or
starts with "logd." and skips dumping logs in this case.
2) It does not dump logs if it is running in process for any process.
Calling alarm() or modifying signal handlers from general purpose
libraries is not recommended either, so without a strong reason to
keep this, this change removes it.
This also shortens the liblog.wrap_mode_blocks test time, since the 30
second issue that it ensures does not happen has been fundamentally
removed.
Test: `kill -8 `pidof logd`` succeeds without delay
Test: liblog, logd unit tests
Change-Id: Id8a40544645d220e49f7ba299201af80a0c44de9
Fix a typo where I check socket()'s return value for 0 instead of -1
as an error.
Remove code that checks if socket() returns 0 as the actual fd that it
returns. This should not happen in any well formed program.
Test: liblog unit tests
Change-Id: I1d878e85d9a39155d68c6c84e9cf9b0db8d1b3a4
We disabled then re-enabled a set of tests believing that they were
stable; they were not, so we disable them again while we investigate
their stability.
A majority of tests filter the logs from logd based on pid, so running
in isolation helps them not see unexpected information from other test
runs.
Bug: 138876729
Bug: 142041379
Test: run via gtest with gtest_repeat=10 without failure
Test: run via atest 5x times without failure
Test: observe that isolated applies to both gtest and atest
Change-Id: I757d52dd3233323be9519625868b2fd2aaa41aeb
These tests are flaky, but we're not seeing what the failure strings
are.
Bug: 142041379
Test: force a failure and see the right error message
Change-Id: Icd7777e5c309cac3b98ce65925980965a3cc3753
The old code did not free the transports, just unlink their nodes from
the linked list, so there was no race condition between close() and
other threads writing to the logs. By settings these to nullptr, I
introduced a race condition, but setting them to nullptr isn't
necessary, so let's simply not do that.
Test: liblog-unit-tests
Change-Id: Iec0b680addd13b5d30bd91ccef5bdeab6bf797b0
The "branchless" macro generates slightly worse code than std::min or
<sys/param.h>'s MIN.
Test: treehugger
Change-Id: I92c17325383bd6116c7b4736d42f01f7a9bb81a3
