On 01/22/2018 11:37 AM, Dan Carpenter wrote:
> The while loop is a post op, "while (i-- >= 0)" so the last iteration
> will read camif_mbus_formats[-1] and then the loop will exit with "i"
> set to -2 and so we do: "mf->code = camif_mbus_formats[-2];".
> 
> I've changed it to a pre-op, I've added a check to ensure we found the
> right format and I've removed the "mf->code = camif_mbus_formats[i];"
> because that is a no-op anyway.
> 
> Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series 
> camera interface")
> Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
> 
> diff --git a/drivers/media/platform/s3c-camif/camif-capture.c 
> b/drivers/media/platform/s3c-camif/camif-capture.c
> index 437395a61065..012f4b389c55 100644
> --- a/drivers/media/platform/s3c-camif/camif-capture.c
> +++ b/drivers/media/platform/s3c-camif/camif-capture.c
> @@ -1261,11 +1261,11 @@ static void __camif_subdev_try_format(struct 
> camif_dev *camif,
>       /* FIXME: constraints against codec or preview path ? */
>       pix_lim = &variant->vp_pix_limits[VP_CODEC];
>   
> -     while (i-- >= 0)
> +     while (--i >= 0)
>               if (camif_mbus_formats[i] == mf->code)
>                       break;
> -
> -     mf->code = camif_mbus_formats[i];
> +     if (i < 0)
> +             return;

Thanks for the patch Dan. mf->width needs to be aligned by this try_format
function so we shouldn't return here. Also it needs to be ensured mf->code 
is set to one of the supported values when this function returns. Sorry,
the current code really doesn't give a clue what was intended.

There is already queued a patch from Arnd [1] addressing the issues you 
have found.
 
>       if (pad == CAMIF_SD_PAD_SINK) {
>               v4l_bound_align_image(&mf->width, 8, CAMIF_MAX_PIX_WIDTH,
> 

[1] https://patchwork.linuxtv.org/patch/46508

--
Regards,
Sylwester

Reply via email to