diff --git a/repos/gems/src/lib/vfs/trace/trace_buffer.h b/repos/gems/src/lib/vfs/trace/trace_buffer.h index 17fc3b0c73..cb6f1e42d2 100644 --- a/repos/gems/src/lib/vfs/trace/trace_buffer.h +++ b/repos/gems/src/lib/vfs/trace/trace_buffer.h @@ -45,31 +45,47 @@ class Trace_buffer if (wrapped) _wrapped_count = _buffer.wrapped(); - /* initialize _curr if _buffer was empty until now */ - Trace::Buffer::Entry curr { _curr }; - if (_curr.last()) - curr = _buffer.first(); + Trace::Buffer::Entry new_curr { _curr }; + Trace::Buffer::Entry entry { _curr }; + + /** + * If '_curr' is marked 'last' (i.e., the entry pointer it contains + * is invalid), either this method wasn't called before on this + * buffer or the buffer was empty on all previous calls (note that + * '_curr' is only updated with valid entry pointers by this + * method). In this case, we start with the first entry (if any). + * + * If '_curr' is not marked 'last' it points to the last processed + * entry and we proceed with the, so far unprocessed entry next to + * it (if any). Note that in this case, the entry behind '_curr' + * might have got overridden since the last call to this method + * because the buffer wrapped and oustripped the entry consumer. + * This problem is well known and should be avoided by choosing a + * large enough buffer. + */ + if (entry.last()) + entry = _buffer.first(); + else + entry = _buffer.next(entry); /* iterate over all entries that were not processed yet */ - Trace::Buffer::Entry e1 = curr; - for (Trace::Buffer::Entry e2 = curr; wrapped || !e2.last(); - e2 = _buffer.next(e2)) { + for (; wrapped || !entry.last(); entry = _buffer.next(entry)) { /* if buffer wrapped, we pass the last entry once and continue at first entry */ - if (wrapped && e2.last()) { + if (wrapped && entry.last()) { wrapped = false; - e2 = _buffer.first(); - if (e2.last()) + entry = _buffer.first(); + if (entry.last()) break; } - e1 = e2; - if (!functor(e1)) + if (!functor(entry)) break; + + new_curr = entry; } /* remember the last processed entry in _curr */ - curr = e1; - if (update) _curr = curr; + if (update) _curr = new_curr; } void * address() const { return &_buffer; } diff --git a/repos/os/src/app/trace_logger/trace_buffer.h b/repos/os/src/app/trace_logger/trace_buffer.h index e0dfb0237b..0830cae728 100644 --- a/repos/os/src/app/trace_logger/trace_buffer.h +++ b/repos/os/src/app/trace_logger/trace_buffer.h @@ -50,28 +50,43 @@ class Trace_buffer _wrapped_count = _buffer.wrapped(); } - /* initialize _curr if _buffer was empty until now */ - if (_curr.last()) - _curr = _buffer.first(); + Trace::Buffer::Entry entry { _curr }; + + /** + * If '_curr' is marked 'last' (i.e., the entry pointer it contains + * is invalid), either this method wasn't called before on this + * buffer or the buffer was empty on all previous calls (note that + * '_curr' is only updated with valid entry pointers by this + * method). In this case, we start with the first entry (if any). + * + * If '_curr' is not marked 'last' it points to the last processed + * entry and we proceed with the, so far unprocessed entry next to + * it (if any). Note that in this case, the entry behind '_curr' + * might have got overridden since the last call to this method + * because the buffer wrapped and oustripped the entry consumer. + * This problem is well known and should be avoided by choosing a + * large enough buffer. + */ + if (entry.last()) + entry = _buffer.first(); + else + entry = _buffer.next(entry); /* iterate over all entries that were not processed yet */ - Trace::Buffer::Entry e1 = _curr; - for (Trace::Buffer::Entry e2 = _curr; wrapped || !e2.last(); - e2 = _buffer.next(e2)) + for (; wrapped || !entry.last(); entry = _buffer.next(entry)) { /* if buffer wrapped, we pass the last entry once and continue at first entry */ - if (wrapped && e2.last()) { + if (wrapped && entry.last()) { wrapped = false; - e2 = _buffer.first(); - if (e2.last()) + entry = _buffer.first(); + if (entry.last()) break; } - e1 = e2; - functor(e1); + /* remember the last processed entry in _curr */ + _curr = entry; + functor(_curr); } - /* remember the last processed entry in _curr */ - _curr = e1; } };