Re: [PATCH v2] iio: chemical: scd30: reject (response=NULL, size>0) in scd30_i2c_command()

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Thu, May 07, 2026 at 08:28:00PM +0500, Stepan Ionichev wrote:
> scd30_i2c_command() takes an opaque "response" buffer plus its size.
> At the start of the function the code already checks if response is
> NULL (via the rsp local), but the response-decoding loop after the
> i2c transfer always dereferences rsp without re-checking. With the
> current callers in scd30_core.c this is harmless, since write
> commands pass response=NULL together with size=0 (so the loop body
> is never entered).
> 
> The (response=NULL, size>0) combination has no useful meaning: there
> is nowhere to put the bytes that come back from the chip. Treat it
> as an invalid argument and bail out at the top of the function with
> -EINVAL, instead of silently doing the i2c transfer and dereferencing
> a NULL pointer in the decode loop.
> 
> smatch flagged the inconsistency:
> 
>   drivers/iio/chemical/scd30_i2c.c:104 scd30_i2c_command() error: we
>     previously assumed rsp could be null (see line 77)
> 
> No functional change for the existing callers, which only ever use
> (response=NULL, size=0) for writes and (response!=NULL, size>0) for
> reads.

Is this analysis AI assisted?

> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
> ---
> v2:
> - Move the check to the top of the function and return -EINVAL on
>   the (response=NULL, size>0) combination, as suggested by Jonathan
>   Cameron. Drop the v1 "if (!rsp) return 0" deeper in the function.

Do not reply to the same email thread with a new patch version.

...

Code wise LGTM, thanks.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko