Skip to content

Commit

Permalink
pw_multisink: Rename argument for better self documentation
Browse files Browse the repository at this point in the history
Renames the drop_count argument to drain_drop_count to better
self document and differentiate from the ingress_drop_count
argument.

Also adds missing *_out suffixes to the ingress drop count
output arguments.

Change-Id: I4b50c898b3ef4c4ed5cf94b52a939775aff5272b
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/95369
Commit-Queue: Auto-Submit <[email protected]>
Reviewed-by: Carlos Chinchilla <[email protected]>
Pigweed-Auto-Submit: Ewout van Bekkum <[email protected]>
  • Loading branch information
Ewout van Bekkum authored and CQ Bot Account committed May 20, 2022
1 parent 65193ef commit 9e7dc9c
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 22 deletions.
25 changes: 13 additions & 12 deletions pw_multisink/multisink.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,12 @@ Result<ConstByteSpan> MultiSink::PeekOrPopEntry(
Drain& drain,
ByteSpan buffer,
Request request,
uint32_t& drop_count_out,
uint32_t& drain_drop_count_out,
uint32_t& ingress_drop_count_out,
uint32_t& entry_sequence_id_out) {
size_t bytes_read = 0;
entry_sequence_id_out = 0;
drop_count_out = 0;
drain_drop_count_out = 0;
ingress_drop_count_out = 0;

std::lock_guard lock(lock_);
Expand Down Expand Up @@ -108,18 +108,19 @@ Result<ConstByteSpan> MultiSink::PeekOrPopEntry(
// current and last sequence IDs. Consecutive successful reads will always
// differ by one at least, so it is subtracted out. If the read was not
// successful, the difference is not adjusted.
drop_count_out = entry_sequence_id_out - drain.last_handled_sequence_id_ -
(peek_status.ok() ? 1 : 0);
drain_drop_count_out = entry_sequence_id_out -
drain.last_handled_sequence_id_ -
(peek_status.ok() ? 1 : 0);

// Only report the ingress drop count when the drain catches up to where the
// drop happened, accounting only for the drops found and no more, as
// indicated by the gap in sequence IDs.
if (drop_count_out > 0) {
if (drain_drop_count_out > 0) {
ingress_drop_count_out =
std::min(drop_count_out,
std::min(drain_drop_count_out,
total_ingress_drops_ - drain.last_handled_ingress_drop_count_);
// Remove the ingress drop count duplicated in drop_count_out.
drop_count_out -= ingress_drop_count_out;
// Remove the ingress drop count duplicated in drain_drop_count_out.
drain_drop_count_out -= ingress_drop_count_out;
// Check if all the ingress drops were reported.
drain.last_handled_ingress_drop_count_ =
total_ingress_drops_ > ingress_drop_count_out
Expand Down Expand Up @@ -225,15 +226,15 @@ Status MultiSink::Drain::PopEntry(const PeekedEntry& entry) {

Result<MultiSink::Drain::PeekedEntry> MultiSink::Drain::PeekEntry(
ByteSpan buffer,
uint32_t& drop_count_out,
uint32_t& drain_drop_count_out,
uint32_t& ingress_drop_count_out) {
PW_DCHECK_NOTNULL(multisink_);
uint32_t entry_sequence_id_out;
Result<ConstByteSpan> peek_result =
multisink_->PeekOrPopEntry(*this,
buffer,
Request::kPeek,
drop_count_out,
drain_drop_count_out,
ingress_drop_count_out,
entry_sequence_id_out);
if (!peek_result.ok()) {
Expand All @@ -244,14 +245,14 @@ Result<MultiSink::Drain::PeekedEntry> MultiSink::Drain::PeekEntry(

Result<ConstByteSpan> MultiSink::Drain::PopEntry(
ByteSpan buffer,
uint32_t& drop_count_out,
uint32_t& drain_drop_count_out,
uint32_t& ingress_drop_count_out) {
PW_DCHECK_NOTNULL(multisink_);
uint32_t entry_sequence_id_out;
return multisink_->PeekOrPopEntry(*this,
buffer,
Request::kPop,
drop_count_out,
drain_drop_count_out,
ingress_drop_count_out,
entry_sequence_id_out);
}
Expand Down
20 changes: 10 additions & 10 deletions pw_multisink/public/pw_multisink/multisink.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class MultiSink {
// drop count in parallel.
//
// If the read operation was successful or returned OutOfRange (i.e. no
// entries to read) then the `drop_count_out` is set to the number of
// entries to read) then the `drain_drop_count_out` is set to the number of
// entries that were dropped since the last call to PopEntry due to
// advancing the drain, and `ingress_drop_count_out` is set to the number of
// logs that were dropped before being added to the MultiSink. Otherwise,
Expand Down Expand Up @@ -133,8 +133,8 @@ class MultiSink {
// RESOURCE_EXHAUSTED - The provided buffer was not large enough to store
// the next available entry, which was discarded.
Result<ConstByteSpan> PopEntry(ByteSpan buffer,
uint32_t& drop_count_out,
uint32_t& ingress_drop_count)
uint32_t& drain_drop_count_out,
uint32_t& ingress_drop_count_out)
PW_LOCKS_EXCLUDED(multisink_->lock_);
// Overload that combines drop counts.
// TODO(cachinchilla): remove when downstream projects migrated to new API.
Expand Down Expand Up @@ -180,8 +180,8 @@ class MultiSink {
// count, without moving the drain forward, except if there is a
// RESOURCE_EXHAUSTED error when peeking, in which case the drain is
// automatically advanced.
// The `drop_count_out` follows the same logic as `PopEntry`. The user must
// call `PopEntry` once the data in peek was used successfully.
// The `drain_drop_count_out` follows the same logic as `PopEntry`. The user
// must call `PopEntry` once the data in peek was used successfully.
//
// Precondition: the buffer data must not be corrupt, otherwise there will
// be a crash.
Expand All @@ -193,8 +193,8 @@ class MultiSink {
// RESOURCE_EXHAUSTED - The provided buffer was not large enough to store
// the next available entry, which was discarded.
Result<PeekedEntry> PeekEntry(ByteSpan buffer,
uint32_t& drop_count_out,
uint32_t& ingress_drop_count)
uint32_t& drain_drop_count_out,
uint32_t& ingress_drop_count_out)
PW_LOCKS_EXCLUDED(multisink_->lock_);

// Drains are not copyable or movable.
Expand Down Expand Up @@ -397,16 +397,16 @@ class MultiSink {
//
// Returns:
// OK - An entry was successfully read from the multisink. The
// `drop_count_out` is set to the difference between the current sequence ID
// and the last handled ID.
// `drain_drop_count_out` is set to the difference between the current
// sequence ID and the last handled ID.
// FAILED_PRECONDITION - The drain is not attached to
// a multisink.
// RESOURCE_EXHAUSTED - The provided buffer was not large enough to store
// the next available entry, which was discarded.
Result<ConstByteSpan> PeekOrPopEntry(Drain& drain,
ByteSpan buffer,
Request request,
uint32_t& drop_count_out,
uint32_t& drain_drop_count_out,
uint32_t& ingress_drop_count_out,
uint32_t& entry_sequence_id_out)
PW_LOCKS_EXCLUDED(lock_);
Expand Down

0 comments on commit 9e7dc9c

Please sign in to comment.