Re: [PATCH] media: atomisp: ov2722: flush buffered writes before they overflow

[Dan Carpenter]

On Mon, Mar 23, 2026 at 08:17:30PM +0800, Pengpeng Hou wrote:
> __ov2722_buf_reg_array() appends 8-bit or 16-bit values to the buffered
> register-write payload and only checks whether it should flush after the
> new value has already been written. When ctrl->index points at the last
> byte of the fixed 30-byte data buffer and the next register is 16-bit,
> the helper writes one byte past the end of the local buffer before the
> flush threshold check runs.
> 
> Check whether the next value fits before writing it. If not, flush the
> current buffered write first and then append the new value.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@xxxxxxxxxxx>

The patch is wrong and just adds dead code.

The function is called in a loop.  The buffer has 30 bytes.  We
write either 1 or 2 bytes.  When the index reaches 28 at the end
of the function then we write the buffer and reset the ctrl->index to
zero.  So the new check for if ctrl->index is larger than 28 or 29
at the start of the function will never trigger.

We never actually write to the last two bytes in the array.  So the
original code is off by one in that sense, I suppose?  The >= should
be > as you wrote your code.  Not a huge deal?

This patch has no information about how the bug was identified or how
the patch tested.  If you put a note to say that "This was found
via static analysis" it changes how we review the code.  Or a note to
say that "this has not been tested" then that also is good information
for reviewers.

There is also no Fixes tag.  Adding a Fixes tag helps you figure out
how the bug was introduced or what the original author may have been
thinking.  It's also necessary for the review process and the stable
process.

regards,
dan carpenter