On Sun, 11 Oct 2020 14:41:20 +0200
Lino Sanfilippo <LinoSanfilippo@xxxxxx> wrote:
> In function map_array_register() properly rewind in case of error.
> Furthermore save an extra label by using a break instead of goto to leave
> the concerning loop.
>
> Signed-off-by: Lino Sanfilippo <LinoSanfilippo@xxxxxx>
Good spot. I'd rather we went with a different code flow though.
See below.
Thanks,
Jonathan
> ---
> drivers/iio/inkern.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 5a8351c..0735cc4 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -28,6 +28,7 @@ int iio_map_array_register(struct iio_dev *indio_dev, struct iio_map *maps)
> {
> int i = 0, ret = 0;
> struct iio_map_internal *mapi;
> + struct list_head *pos, *tmp;
>
> if (maps == NULL)
> return 0;
> @@ -37,14 +38,22 @@ int iio_map_array_register(struct iio_dev *indio_dev, struct iio_map *maps)
> mapi = kzalloc(sizeof(*mapi), GFP_KERNEL);
> if (mapi == NULL) {
> ret = -ENOMEM;
> - goto error_ret;
> + break;
> }
> mapi->map = &maps[i];
> mapi->indio_dev = indio_dev;
> list_add_tail(&mapi->l, &iio_map_list);
> i++;
> }
> -error_ret:
> +
Please do this as a a separate error path rather than having
it as non trivial unwinding in the main code flow.
i.e. goto error_ret, but handle that path separately after
the return below.
> + if (ret) { /* undo */
> + while (i--) {
> + mapi = list_last_entry(&iio_map_list,
> + struct iio_map_internal, l);
> + list_del(&mapi->l);
> + kfree(mapi);
> + }
> + }
> mutex_unlock(&iio_map_list_lock);
>
> return ret;
> --
> 2.7.4
>
