Hi Bingbu, Arnd,
On Tue, Mar 05, 2019 at 12:25:18AM +0000, Cao, Bingbu wrote:
...
> > @@ -1744,7 +1745,7 @@ int imgu_css_fmt_try(struct imgu_css *css,
> > struct v4l2_rect *const bds = &r[IPU3_CSS_RECT_BDS];
> > struct v4l2_rect *const env = &r[IPU3_CSS_RECT_ENVELOPE];
> > struct v4l2_rect *const gdc = &r[IPU3_CSS_RECT_GDC];
> > - struct imgu_css_queue q[IPU3_CSS_QUEUES];
> > + struct imgu_css_queue *q = kcalloc(IPU3_CSS_QUEUES, sizeof(struct
> > +imgu_css_queue), GFP_KERNEL);
>
> Could you use the devm_kcalloc()?
No, because this is not related to the device, called instead on
e.g. VIDIOC_TRY_FMT.
> > struct v4l2_pix_format_mplane *const in =
> > &q[IPU3_CSS_QUEUE_IN].fmt.mpix;
> > struct v4l2_pix_format_mplane *const out = @@ -1753,6 +1754,11 @@
> > int imgu_css_fmt_try(struct imgu_css *css,
> > &q[IPU3_CSS_QUEUE_VF].fmt.mpix;
> > int i, s, ret;
> >
> > + if (!q) {
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> [Cao, Bingbu]
> The goto here is wrong, you can just report an error, and I prefer it is next
> to the alloc.
I agree, the goto is just not needed.
> > +
> > /* Adjust all formats, get statistics buffer sizes and formats */
> > for (i = 0; i < IPU3_CSS_QUEUES; i++) {
> > if (fmts[i])
> > @@ -1766,7 +1772,8 @@ int imgu_css_fmt_try(struct imgu_css *css,
> > IPU3_CSS_QUEUE_TO_FLAGS(i))) {
> > dev_notice(css->dev, "can not initialize queue %s\n",
> > qnames[i]);
> > - return -EINVAL;
> > + ret = -EINVAL;
> > + goto out;
> > }
> > }
> > for (i = 0; i < IPU3_CSS_RECTS; i++) { @@ -1788,7 +1795,8 @@ int
> > imgu_css_fmt_try(struct imgu_css *css,
> > if (!imgu_css_queue_enabled(&q[IPU3_CSS_QUEUE_IN]) ||
> > !imgu_css_queue_enabled(&q[IPU3_CSS_QUEUE_OUT])) {
> > dev_warn(css->dev, "required queues are disabled\n");
> > - return -EINVAL;
> > + ret = -EINVAL;
> > + goto out;
> > }
> >
> > if (!imgu_css_queue_enabled(&q[IPU3_CSS_QUEUE_OUT])) { @@ -1829,7
> > +1837,8 @@ int imgu_css_fmt_try(struct imgu_css *css,
> > ret = imgu_css_find_binary(css, pipe, q, r);
> > if (ret < 0) {
> > dev_err(css->dev, "failed to find suitable binary\n");
> > - return -EINVAL;
> > + ret = -EINVAL;
> > + goto out;
> > }
> > css->pipes[pipe].bindex = ret;
> >
> > @@ -1843,7 +1852,8 @@ int imgu_css_fmt_try(struct imgu_css *css,
> > IPU3_CSS_QUEUE_TO_FLAGS(i))) {
> > dev_err(css->dev,
> > "final resolution adjustment failed\n");
> > - return -EINVAL;
> > + ret = -EINVAL;
> > + goto out;
> > }
> > *fmts[i] = q[i].fmt.mpix;
> > }
> > @@ -1859,7 +1869,10 @@ int imgu_css_fmt_try(struct imgu_css *css,
> > bds->width, bds->height, gdc->width, gdc->height,
> > out->width, out->height, vf->width, vf->height);
> >
> > - return 0;
> > + ret = 0;
> > +out:
> > + kfree(q);
> > + return ret;
> > }
> >
> > int imgu_css_fmt_set(struct imgu_css *css,
--
Regards,
Sakari Ailus
sakari.ai...@linux.intel.com
