On Fri, Jun 17, 2022 at 06:49:41PM +0100, Jonathan Cameron wrote:
> > +static irqreturn_t bno08x_trigger_handler(int irq, void *p)
> > +{
> > + struct iio_poll_func *pf = p;
> > + struct iio_dev *iio_dev = pf->indio_dev;
> > + struct bno08x_dev *bno08x = iio_priv(iio_dev);
> > + u8 cargo[BNO08x_CARGO_BUFFER_SIZE];
> > + int ret;
> > +
> > + ret = bno08x_wait_for_cargo_timeout(bno08x, cargo, BNO08x_CARGO_BUFFER_SIZE);
> > + if (ret < 0)
> > + goto done;
> > +
> I'm lazy - if it's less than 0 it's definitely less than 24 so no need for separate
> paths unless you add some dev_dbg/err or similar which might make sense here.
>
Also 24 is a magic number. But you have to be a bit careful with
combining conditions. The two common bugs are forgot to set the error
code and type promotion to unsigned long:
if (ret < sizeof(buf))
goto done;
Negative ret values are > sizeof(buf) and for 0 to sizeof - 1 then the
error code isn't set so this made up example would be totally broken.
None of that applies here, of course, but I like breaking the conditions
apart just as a habit.
> > + if (ret < 24)
> > + goto done;
> > +
regards,
dan carpenter
