fix(streaming): keep the sign when counting samples until the buffer end

[nicolas-rabault]

Stacked on #509, which is stacked on #508. Base is fix/streaming-wire-size-in-bytes so the diff shows only this change. Merge #508 → #509 → this.

Third and last of the streaming sweep. One real bug plus two things that cannot change behaviour.

1. Sign lost when counting samples until the buffer end

streaming.c:154 divided the pointer difference before narrowing it to int32_t:

int32_t nb_available_sample = ((uintptr_t)data_ptr - (uintptr_t)sample_ptr) / data_size;
if (nb_available_sample < 0) { /* the buffer have looped */ }

(uintptr_t)a - (uintptr_t)b is unsigned arithmetic. On a looped buffer the difference is a near-UINTPTR_MAX value, and dividing that by data_size yields a large positive number — so the loop branch is never taken and the function returns garbage instead of the samples left before the end of the ring.

Streaming_GetAvailableSampleNB immediately above already casts first (line 132). This just makes the two consistent.

Why it was invisible

Every channel in the tree uses data_size = 1 — pipe.c creates both of its channels that way, and so did every test. Dividing by 1 leaves the bit pattern untouched, so the sign survives and the bug cannot show.

It is not a 32-bit-only problem either, which is what I first assumed. With a data_size that is not a power of two the sign is lost on any word size:

32-bit, data_size=3: cast-first=-1 (<0? YES)  divide-first=1431655764 (<0? NO)
64-bit, data_size=3: cast-first=-1 (<0? YES)  divide-first=1431655764 (<0? NO)

So the added test case uses a 10-sample, 3-byte channel and runs on the native 64-bit suite. Verified: it reports Expected 1 Was 1431655764 against the unfixed code.

Blast radius

The callers are the pipe services (SERIAL, WS/ESP32_IDF, WS/native, WS/ARDUINO), which use the result to size their reads. All are data_size = 1, so no shipped configuration changes behaviour. This closes a trap for the first data_size > 1 channel that calls it rather than fixing a live failure.

2. Unreachable branch in Streaming_RmvAvailableSampleNB

stream->sample_ptr = sample_ptr + (size * data_size);
if (stream->sample_ptr == stream->end_ring_buffer)   // can never be true here

The else is entered only when sample_ptr + size * data_size < end_ring_buffer, so the new pointer is always strictly below the end. The branch above already handles landing exactly on it (chunk1 == size * data_size → chunk2 == 0 → sample_ptr = ring_buffer), which the existing "buffer loop case" test covers. Removed.

3. Dead assignment in Streaming_GetSample

data = memcpy(data, stream->sample_ptr, ...) assigned the return value onto the function's own data parameter. Removed the assignment, kept the memcpy.

Tests

124 test cases: 124 succeeded    (#509 branch, baseline)
124 test cases: 124 succeeded    (this branch — new case added to an existing test function)

The new case fails on the unfixed code. No existing test modified. clang-format clean.

Deliberately left alone

Two things from the same review that are not no-risk, so they are not here:

• Streaming_GetSample returns 0 both for "read everything, buffer now empty" and for "nothing available". Callers cannot tell them apart. Fixing it means changing a public return contract; Luos_SendStreamingSize ignores the value so nothing is broken today.
• Streaming_AddAvailableSampleNB refuses to fill a channel exactly (> 0) while Streaming_PutSample allows it (<= capacity). They disagree by one. An existing test pins PutSample's behaviour, so picking a side is an API decision, not a cleanup.

Also noted and not touched: a channel left zero-initialised and fed a message before Streaming_CreateChannel divides by data_size == 0. Adding an assert there would be idiomatic but could newly fire in user code.