| Message ID | 1527583688-314-5-git-send-email-jacopo+renesas@jmondi.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Jacopo, Thankyou for the patch, On 29/05/18 09:48, Jacopo Mondi wrote: > During the notifier initialization, memory for the list of associated async > subdevices is reserved during the fwnode endpoint parsing from the v4l2-async > framework. If the notifier registration fails, that memory should be released > and the notifier 'cleaned up'. > > Catch the notifier registration error and perform the cleanup both for the > group and the parallel notifiers. > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > --- > v5: > - new patch > > --- > drivers/media/platform/rcar-vin/rcar-core.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c > index d3aadf3..f7a28e9 100644 > --- a/drivers/media/platform/rcar-vin/rcar-core.c > +++ b/drivers/media/platform/rcar-vin/rcar-core.c > @@ -573,10 +573,15 @@ static int rvin_parallel_graph_init(struct rvin_dev *vin) > ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier); > if (ret < 0) { > vin_err(vin, "Notifier registration failed\n"); > - return ret; > + goto error_notifier_cleanup; > } > > return 0; > + > +error_notifier_cleanup: > + v4l2_async_notifier_cleanup(&vin->group->notifier); Wouldn't it be less lines to just call the cleanup before the return? Or do you anticipate multiple paths needing to call through the clean up here ? > + > + return ret; > } > > /* ----------------------------------------------------------------------------- > @@ -775,10 +780,15 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin) > &vin->group->notifier); > if (ret < 0) { > vin_err(vin, "Notifier registration failed\n"); > - return ret; > + goto error_notifier_cleanup; > } > > return 0; > + > +error_notifier_cleanup: > + v4l2_async_notifier_cleanup(&vin->group->notifier); > + Same here... > + return ret; > } > > static int rvin_mc_init(struct rvin_dev *vin) > -- > 2.7.4 >
Hi Jacopo, Thanks for your work. On 2018-05-29 10:48:02 +0200, Jacopo Mondi wrote: > During the notifier initialization, memory for the list of associated async > subdevices is reserved during the fwnode endpoint parsing from the v4l2-async > framework. If the notifier registration fails, that memory should be released > and the notifier 'cleaned up'. > > Catch the notifier registration error and perform the cleanup both for the > group and the parallel notifiers. > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> I agree with Kieran's review comment that it's better to call v4l2_async_notifier_cleanup() directly instead of adding a goto. With that fixed feel free to add Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > --- > v5: > - new patch > > --- > drivers/media/platform/rcar-vin/rcar-core.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c > index d3aadf3..f7a28e9 100644 > --- a/drivers/media/platform/rcar-vin/rcar-core.c > +++ b/drivers/media/platform/rcar-vin/rcar-core.c > @@ -573,10 +573,15 @@ static int rvin_parallel_graph_init(struct rvin_dev *vin) > ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier); > if (ret < 0) { > vin_err(vin, "Notifier registration failed\n"); > - return ret; > + goto error_notifier_cleanup; > } > > return 0; > + > +error_notifier_cleanup: > + v4l2_async_notifier_cleanup(&vin->group->notifier); > + > + return ret; > } > > /* ----------------------------------------------------------------------------- > @@ -775,10 +780,15 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin) > &vin->group->notifier); > if (ret < 0) { > vin_err(vin, "Notifier registration failed\n"); > - return ret; > + goto error_notifier_cleanup; > } > > return 0; > + > +error_notifier_cleanup: > + v4l2_async_notifier_cleanup(&vin->group->notifier); > + > + return ret; > } > > static int rvin_mc_init(struct rvin_dev *vin) > -- > 2.7.4 >
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c index d3aadf3..f7a28e9 100644 --- a/drivers/media/platform/rcar-vin/rcar-core.c +++ b/drivers/media/platform/rcar-vin/rcar-core.c @@ -573,10 +573,15 @@ static int rvin_parallel_graph_init(struct rvin_dev *vin) ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier); if (ret < 0) { vin_err(vin, "Notifier registration failed\n"); - return ret; + goto error_notifier_cleanup; } return 0; + +error_notifier_cleanup: + v4l2_async_notifier_cleanup(&vin->group->notifier); + + return ret; } /* ----------------------------------------------------------------------------- @@ -775,10 +780,15 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin) &vin->group->notifier); if (ret < 0) { vin_err(vin, "Notifier registration failed\n"); - return ret; + goto error_notifier_cleanup; } return 0; + +error_notifier_cleanup: + v4l2_async_notifier_cleanup(&vin->group->notifier); + + return ret; } static int rvin_mc_init(struct rvin_dev *vin)
During the notifier initialization, memory for the list of associated async subdevices is reserved during the fwnode endpoint parsing from the v4l2-async framework. If the notifier registration fails, that memory should be released and the notifier 'cleaned up'. Catch the notifier registration error and perform the cleanup both for the group and the parallel notifiers. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- v5: - new patch --- drivers/media/platform/rcar-vin/rcar-core.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) -- 2.7.4