Re: [PATCH RFC v6 4/5] iio: osf: add authenticated stream parser

[Kim Jinseob]

Understood. I missed applying the earlier style feedback consistently
across this file, and I should have explicitly replied where a previous
comment was not addressed.

I will these:
- reuse the existing OSF magic definition instead of duplicating a byte array
  in the stream parser;
- use loop-local variables where the variable is not used outside the loop;
- rework the stream error handling so authenticated and unauthenticated
  failures are handled deliberately;
- either update partial_frames where appropriate or remove the unused
  statistic.

I will also check lore for the previous versions and track the earlier review
comments before posting another revision, making sure each one is either
addressed in code or explicitly answered.

Thanks,
Jinseob

2026년 6월 30일 (화) 오전 8:06, Jonathan Cameron <jic23@xxxxxxxxxx>님이 작성:
>
> On Mon, 29 Jun 2026 17:10:31 +0300
> Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:
>
> > On Mon, Jun 29, 2026 at 04:13:36AM +0900, Jinseob Kim wrote:
> > > Add a UART byte-stream parser for Open Sensor Fusion frames.
> > >
> > > The parser searches for the OSF0 wire magic, keeps partial frames
> > > buffered, checks header length and payload bounds, and passes complete
> > > candidate frames to the core decoder.
> > >
> > > Rejected candidate frames drop only the current head byte before
> > > resynchronizing, so a corrupted unauthenticated payload length cannot
> > > make the parser skip later valid frames.
> >
> > ...
> >
> > > +#define OSF_STREAM_MAGIC_LEN       4
> > > +#define OSF_STREAM_MAX_PAYLOAD_LEN                         \
> > > +   (OSF_STREAM_MAX_FRAME_LEN - OSF_FRAME_HEADER_LEN - OSF_FRAME_CRC_LEN)
> > > +
> > > +static const u8 osf_stream_magic[OSF_STREAM_MAGIC_LEN] = {
> > > +   'O', 'S', 'F', '0',
> > > +};
> >
> > You have already this in the header (as FourCC), use that.
> >
> > ...
> >
> > > +static size_t osf_stream_discard_to_magic(struct osf_stream *stream)
> > > +{
> > > +   size_t old_len = stream->len;
> > > +   size_t match_len;
> >
> > > +   size_t i;
> > > +
> > > +   for (i = 0; i < stream->len; i++) {
> >
> >       for (size_t i = 0; i < stream->len; i++) {
> >
> > > +           match_len = stream->len - i;
> > > +           if (match_len > OSF_STREAM_MAGIC_LEN)
> > > +                   match_len = OSF_STREAM_MAGIC_LEN;
> > > +
> > > +           if (osf_stream_magic_match(stream->buf + i, match_len)) {
> > > +                   if (i)
> > > +                           osf_stream_discard(stream, i);
> > > +                   return i;
> > > +           }
> > > +   }
> > > +
> > > +   stream->len = 0;
> > > +   return old_len;
> > > +}
> >
> > ...
> >
> > I stop here, because it's obvious that you neglected and ignored my previous
> > reviews. No explanation given, nothing. This is not how you should interact
> > with the community.
> >
> > Come again when each of the given comment will be either addressed or argued.
> >
> Likewise.  Please make sure to address every comment either through
> changes or through reply to the earlier thread.  Perhaps some emails
> have gone astray (it happens!). It can be a good idea to take a quick
> look at lore.kernel.org to make sure you aren't missing any feedback
> on a previous version.
>
> Thanks,
>
> Jonathan
>
>