Clamp the overwrite discard to what the buffer holds

[youdie006]

In overwrite mode, write() makes room by advancing the read pointer (ring_buffer.go:664):

needed := len(p) - avail
r.r = (r.r + needed) % r.size

When len(p) is larger than the buffer, needed is bigger than the data actually present, and % r.size wraps it to an arbitrary offset rather than clamping. The read pointer ends up past space that was never written.

rb := ringbuffer.New(4).SetOverwrite(true)
n, err := rb.Write([]byte("abcdef"))
// n = 2, Bytes(nil) = "\x00\x00ab", Length() = 4

Two NUL bytes that were never written are readable, and Length() reports a full buffer.

Measured at master, four buffer states:

empty     + "abcdef"  -> n=2  Bytes="\x00\x00ab"
empty     + "abcde"   -> n=1  Bytes="\x00\x00\x00a"
"ab"      + "cdefgh"  -> n=2  Bytes="\x00\x00cd"
"abcd"    + "efghij"  -> n=2  Bytes="cdef"

The last row still has "cd" from the previous write.

Your own test already declares the right answer

TestRingBuffer_OverwriteMode_TwiceFull (ring_buffer_test.go:1930) writes 8 bytes into a full 4-byte buffer and asserts "efgh" -- an oversize write keeps the first size bytes and drops everything older. That is the semantic I am matching.

It passes today only because 8 % 4 == 0, so the wrap happens to land back on the same offset. Change the 8 to a 6 and the identical call returns 2 bytes with "cd" still readable.

writeByte (ring_buffer.go:764) is the other half of the pair: it advances r.r only inside if r.w == r.r && r.isFull, so it never moves the read pointer past data that exists. Only the bulk path does.

The fix

Clamp needed to r.size - avail, the amount actually held. After it, all four rows above give n=4 and agree with TwiceFull: "abcd", "abcd", "cdef", "efgh".

What I deliberately did not change

The semantic for an oversize write stays yours -- the first size bytes are kept and ErrTooMuchDataToWrite is still returned. Making it keep the newest size bytes instead would be a design proposal and would contradict TwiceFull. This only makes every buffer state produce the answer that test already declares.

I also left alone the fact that a blocking overwrite Write waits on ErrTooMuchDataToWrite (ring_buffer.go:414); that is a separate question.

Verification

go test -race -count=1 ./... (the workflow's line) -- ok github.com/smallnest/ringbuffer 21.960s. gofmt -l . empty, go vet ./... clean. TestIssue23, the regression test from the closed issue in the same overwrite family, passes.

I checked the clamp from three directions rather than only confirming the new rows go green:

• drop the clamp -- the new test fails on all four rows
• r.size - avail - 1 -- your TwiceFull fails, Expected 'efgh', got 'defg'
• r.size - avail + 1 -- your TwiceFull fails, Expected 'efgh', got 'bcde'

Your existing row failing on both sides is what pins the value exactly, and it means this adds a clamp arm rather than moving a threshold.

Observable output changes only for len(p) > Capacity() in overwrite mode. The whole suite, including all eight OverwriteMode_* tests, is green unmodified -- no existing assertion had to change.

---

Disclosure: AI-assisted. I found and prepared this with an AI assistant, and I ran and verified everything above myself.

[smallnest]

Review: LGTM — merging

I reviewed the diff, audited the overwrite path in write(), and ran the suite locally (go test -race ./..., plus all OverwriteMode_* and TestIssue23) — all green, go.mod untouched.

The bug is real

In overwrite mode, write() makes room via needed = len(p) - avail then r.r = (r.r + needed) % r.size. When len(p) exceeds the whole buffer, needed is larger than the data actually held, so the modulo lands the read pointer on an arbitrary offset past never-written space. Result: NUL bytes become readable and Length() falsely reports full (New(4).SetOverwrite(true).Write("abcdef") → n=2, Bytes="\x00\x00ab").

The fix is correct and minimal

Clamping needed to r.size - avail (the amount actually held) caps the read-pointer advance at r.w, i.e. the buffer becomes empty rather than wrapping into garbage. I hand-verified all four buffer states; each now yields a consistent result that matches the semantic already asserted by TestRingBuffer_OverwriteMode_TwiceFull (that test only passed before because 8 % 4 == 0 happened to cancel the wrap).

Scope is clean

• Design semantics preserved on purpose: an oversize write still keeps the first size bytes and still returns ErrTooMuchDataToWrite. This is a bug fix, not a behavior change.
• No existing assertion had to change; observable output changes only for len(p) > Capacity() in overwrite mode.
• Well-chosen table-driven test covering empty/odd/partial/full states.

Nice, tightly-scoped fix. Merging.