Re: [PATCH RFC v2 4/7] iio: osf: add stream parser

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The 'partial_frames' statistic is incremented erroneously during normal chunked data reception, making the metric artificially spammed and useless.
- [Low] The frame is successfully parsed into a stack variable, but the populated structure is ignored and completely unused.
--

commit f278438df751263938f58f94af6a218883f9f11f
Author: Jinseob Kim <kimjinseob88@gmail.com>

iio: osf: add stream parser

This commit introduces byte stream assembly and frame synchronization
capabilities for OSF0 frames. It handles buffering, magic sequence matching,
and payload validation before passing frames to the core.

> diff --git a/drivers/iio/opensensorfusion/osf_stream.c b/drivers/iio/opensensorfusion/osf_stream.c
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_stream.c

[ ... ]

> +static int osf_stream_process(struct osf_stream *stream)
> +{
> +	struct osf_frame frame;
> +	size_t decoded_len;
> +	size_t discarded;
> +	size_t frame_len;
> +	u32 payload_len;
> +	int first_err = 0;
> +	int ret;
> +
> +	while (stream->len) {
> +		discarded = osf_stream_discard_to_magic(stream);

[ ... ]

> +		if (!stream->len)
> +			break;
> +
> +		if (stream->len < OSF_FRAME_HEADER_LEN) {
> +			stream->stats.partial_frames++;

[Severity: Medium]
Is it intentional to increment this statistic during normal chunked data
reception? 

Since stream data often arrives in partial fragments, breaking here is expected
behavior while waiting for the rest of the stream. Incrementing an error
counter on every normal yield might spam the metric and make it less useful
for debugging actual issues.

> +			break;
> +		}
> +
> +		if (get_unaligned_le16(stream->buf + 6) !=

[ ... ]

> +		frame_len = OSF_FRAME_HEADER_LEN + payload_len + OSF_FRAME_CRC_LEN;
> +		if (stream->len < frame_len) {
> +			stream->stats.partial_frames++;

[Severity: Medium]
Similarly, does this falsely increment the partial_frames statistic while
simply waiting for the rest of a valid payload to arrive?

> +			break;
> +		}
> +
> +		ret = osf_protocol_decode_frame(stream->buf, frame_len, &frame,
> +						&decoded_len);

[ ... ]

> +		if (decoded_len != frame_len) {
> +			stream->stats.dropped_bytes++;
> +			osf_stream_drop_invalid_head(stream);
> +			if (!first_err)
> +				first_err = -EMSGSIZE;
> +			continue;
> +		}
> +
> +		ret = osf_core_receive_frame(stream->osf, stream->buf, frame_len);

[Severity: Low]
Does this code need to pass the raw buffer to osf_core_receive_frame()
instead of the parsed struct osf_frame?

The earlier call to osf_protocol_decode_frame() successfully parses the frame
into the stack variable frame, but it appears to go unused. Passing the
already-parsed structure to the core might avoid redundant decoding
inefficiency.

> +		if (ret) {
> +			osf_stream_discard(stream, frame_len);
> +			if (!first_err)
> +				first_err = ret;
> +			continue;
> +		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260524085312.15369-1-kimjinseob88@gmail.com?part=4