Re: [PATCH 1/3] media: staging: rkisp1: params: don't reference the vb2 buffer after calling vb2_buffer_done

[Helen Koike <helen.koike@xxxxxxxxxxxxx>]

Hi Dafna,

Thanks for your patch.

On 6/25/20 2:42 PM, Dafna Hirschfeld wrote:
> The driver should not reference the buffer pointer of vb2_buffer
> after calling 'vb2_buffer_done' on that buffer since the call passes
> the buffer to userspace.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@xxxxxxxxxxxxx>
> ---
>  drivers/staging/media/rkisp1/rkisp1-params.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
> index 797e79de659c..762c2259b807 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-params.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-params.c
> @@ -1457,9 +1457,9 @@ static void rkisp1_params_vb2_buf_queue(struct vb2_buffer *vb)
>  		new_params = (struct rkisp1_params_cfg *)
>  			(vb2_plane_vaddr(vb, 0));
>  		vbuf->sequence = frame_sequence;
> -		vb2_buffer_done(&params_buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
>  		params->is_first_params = false;
>  		params->cur_params = *new_params;

Maybe we can remove this new_params variable entirely, and just copy directly to params->cur_params in the begining of the if statement, what do yo think?

With or without this change

Acked-by: Helen Koike <helen.koike@xxxxxxxxxxxxx>

Thanks
Helen

> +		vb2_buffer_done(&params_buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
>  		return;>  	}
>  
>