Hi Ricardo,
Thank you for the patch.
On Tue, Jan 03, 2023 at 03:36:25PM +0100, Ricardo Ribalda wrote:
> Simplify the exit code with a common error tag freeing all the memory.
>
> Suggested-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 26 +++++++++++++++-----------
> 1 file changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index aa7a668f60a7..4830120e6506 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2296,32 +2296,30 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
> unsigned int i;
>
> /*
> - * Most mappings come from static kernel data and need to be duplicated.
> + * Most mappings come from static kernel data, and need to be duplicated.
> * Mappings that come from userspace will be unnecessarily duplicated,
> * this could be optimized.
> */
> map = kmemdup(mapping, sizeof(*mapping), GFP_KERNEL);
> - if (map == NULL)
> + if (!map)
> return -ENOMEM;
>
> + map->name = NULL;
> + map->menu_info = NULL;
> +
> /* For UVCIOC_CTRL_MAP custom control */
> if (mapping->name) {
> map->name = kstrdup(mapping->name, GFP_KERNEL);
> - if (!map->name) {
> - kfree(map);
> - return -ENOMEM;
> - }
> + if (!map->name)
> + goto nomem;
> }
>
> INIT_LIST_HEAD(&map->ev_subs);
>
> size = sizeof(*mapping->menu_info) * fls(mapping->menu_mask);
> map->menu_info = kmemdup(mapping->menu_info, size, GFP_KERNEL);
> - if (map->menu_info == NULL) {
> - kfree(map->name);
> - kfree(map);
> - return -ENOMEM;
> - }
> + if (!map->menu_info)
> + goto nomem;
>
> if (map->get == NULL)
> map->get = uvc_get_le_value;
> @@ -2342,6 +2340,12 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
> ctrl->info.selector);
>
> return 0;
> +
> +nomem:
As it's an error label, I'd name it err_nomem. I'll fix that when
applying.
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> + kfree(map->menu_info);
> + kfree(map->name);
> + kfree(map);
> + return -ENOMEM;
> }
>
> int uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
>
--
Regards,
Laurent Pinchart
