[PATCH] media: uvcvideo: Use scope-based cleanup helper

Chen Changcheng posted 1 patch 1 month ago
There is a newer version of this series
drivers/media/usb/uvc/uvc_video.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
[PATCH] media: uvcvideo: Use scope-based cleanup helper
Posted by Chen Changcheng 1 month ago
Replace the manual kfree() with __free(kfree) annotation for data
references. This aligns the code with the latest kernel style.

No functional change intended.

Signed-off-by: Chen Changcheng <chenchangcheng@kylinos.cn>
---
 drivers/media/usb/uvc/uvc_video.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 2094e059d7d3..1ee06d597431 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -291,7 +291,7 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
 	struct uvc_streaming_control *ctrl, int probe, u8 query)
 {
 	u16 size = uvc_video_ctrl_size(stream);
-	u8 *data;
+	u8 *data __free(kfree) = NULL;
 	int ret;
 
 	if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
@@ -317,8 +317,7 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
 			"supported. Enabling workaround.\n");
 		memset(ctrl, 0, sizeof(*ctrl));
 		ctrl->wCompQuality = le16_to_cpup((__le16 *)data);
-		ret = 0;
-		goto out;
+		return 0
 	} else if (query == UVC_GET_DEF && probe == 1 && ret != size) {
 		/*
 		 * Many cameras don't support the GET_DEF request on their
@@ -328,15 +327,13 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
 		uvc_warn_once(stream->dev, UVC_WARN_PROBE_DEF, "UVC non "
 			"compliance - GET_DEF(PROBE) not supported. "
 			"Enabling workaround.\n");
-		ret = -EIO;
-		goto out;
+		return -EIO;
 	} else if (ret != size) {
 		dev_err(&stream->intf->dev,
 			"Failed to query (%s) UVC %s control : %d (exp. %u).\n",
 			uvc_query_name(query), probe ? "probe" : "commit",
 			ret, size);
-		ret = (ret == -EPROTO) ? -EPROTO : -EIO;
-		goto out;
+		return (ret == -EPROTO) ? -EPROTO : -EIO;
 	}
 
 	ctrl->bmHint = le16_to_cpup((__le16 *)&data[0]);
@@ -371,18 +368,15 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
 	 * format and frame descriptors.
 	 */
 	uvc_fixup_video_ctrl(stream, ctrl);
-	ret = 0;
 
-out:
-	kfree(data);
-	return ret;
+	return 0;
 }
 
 static int uvc_set_video_ctrl(struct uvc_streaming *stream,
 	struct uvc_streaming_control *ctrl, int probe)
 {
 	u16 size = uvc_video_ctrl_size(stream);
-	u8 *data;
+	u8 *data __free(kfree) = NULL;
 	int ret;
 
 	data = kzalloc(size, GFP_KERNEL);
@@ -419,7 +413,6 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
 		ret = -EIO;
 	}
 
-	kfree(data);
 	return ret;
 }
 

base-commit: 805f9a061372164d43ddef771d7cd63e3ba6d845
-- 
2.25.1
Re: [PATCH] media: uvcvideo: Use scope-based cleanup helper
Posted by Laurent Pinchart 1 month ago
On Sun, Jan 04, 2026 at 02:45:20PM +0800, Chen Changcheng wrote:
> Replace the manual kfree() with __free(kfree) annotation for data
> references. This aligns the code with the latest kernel style.

There are more occurrences through the driver, please address them all.

> No functional change intended.
> 
> Signed-off-by: Chen Changcheng <chenchangcheng@kylinos.cn>
> ---
>  drivers/media/usb/uvc/uvc_video.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 2094e059d7d3..1ee06d597431 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -291,7 +291,7 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
>  	struct uvc_streaming_control *ctrl, int probe, u8 query)
>  {
>  	u16 size = uvc_video_ctrl_size(stream);
> -	u8 *data;
> +	u8 *data __free(kfree) = NULL;
>  	int ret;
>  
>  	if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
> @@ -317,8 +317,7 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
>  			"supported. Enabling workaround.\n");
>  		memset(ctrl, 0, sizeof(*ctrl));
>  		ctrl->wCompQuality = le16_to_cpup((__le16 *)data);
> -		ret = 0;
> -		goto out;
> +		return 0
>  	} else if (query == UVC_GET_DEF && probe == 1 && ret != size) {
>  		/*
>  		 * Many cameras don't support the GET_DEF request on their
> @@ -328,15 +327,13 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
>  		uvc_warn_once(stream->dev, UVC_WARN_PROBE_DEF, "UVC non "
>  			"compliance - GET_DEF(PROBE) not supported. "
>  			"Enabling workaround.\n");
> -		ret = -EIO;
> -		goto out;
> +		return -EIO;
>  	} else if (ret != size) {
>  		dev_err(&stream->intf->dev,
>  			"Failed to query (%s) UVC %s control : %d (exp. %u).\n",
>  			uvc_query_name(query), probe ? "probe" : "commit",
>  			ret, size);
> -		ret = (ret == -EPROTO) ? -EPROTO : -EIO;
> -		goto out;
> +		return (ret == -EPROTO) ? -EPROTO : -EIO;
>  	}
>  
>  	ctrl->bmHint = le16_to_cpup((__le16 *)&data[0]);
> @@ -371,18 +368,15 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
>  	 * format and frame descriptors.
>  	 */
>  	uvc_fixup_video_ctrl(stream, ctrl);
> -	ret = 0;
>  
> -out:
> -	kfree(data);
> -	return ret;
> +	return 0;
>  }
>  
>  static int uvc_set_video_ctrl(struct uvc_streaming *stream,
>  	struct uvc_streaming_control *ctrl, int probe)
>  {
>  	u16 size = uvc_video_ctrl_size(stream);
> -	u8 *data;
> +	u8 *data __free(kfree) = NULL;
>  	int ret;
>  
>  	data = kzalloc(size, GFP_KERNEL);
> @@ -419,7 +413,6 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
>  		ret = -EIO;

You can return -EIO here...

>  	}
>  
> -	kfree(data);
>  	return ret;

... and replace this with return 0. That's a change in behaviour, but as
far as I can tell, none of the callers use the positive return values, so
returning 0 on success should be fine.

>  }
>  
> 
> base-commit: 805f9a061372164d43ddef771d7cd63e3ba6d845

-- 
Regards,

Laurent Pinchart
Re: [PATCH] media: uvcvideo: Use scope-based cleanup helper
Posted by kernel test robot 1 month ago
Hi Chen,

kernel test robot noticed the following build errors:

[auto build test ERROR on 805f9a061372164d43ddef771d7cd63e3ba6d845]

url:    https://github.com/intel-lab-lkp/linux/commits/Chen-Changcheng/media-uvcvideo-Use-scope-based-cleanup-helper/20260104-144716
base:   805f9a061372164d43ddef771d7cd63e3ba6d845
patch link:    https://lore.kernel.org/r/20260104064520.115462-1-chenchangcheng%40kylinos.cn
patch subject: [PATCH] media: uvcvideo: Use scope-based cleanup helper
config: x86_64-rhel-9.4 (https://download.01.org/0day-ci/archive/20260104/202601041449.BwJ5RZ2a-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260104/202601041449.BwJ5RZ2a-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601041449.BwJ5RZ2a-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/media/usb/uvc/uvc_video.c: In function 'uvc_get_video_ctrl':
>> drivers/media/usb/uvc/uvc_video.c:320:25: error: expected ';' before '}' token
     320 |                 return 0
         |                         ^
         |                         ;
     321 |         } else if (query == UVC_GET_DEF && probe == 1 && ret != size) {
         |         ~                


vim +320 drivers/media/usb/uvc/uvc_video.c

   289	
   290	static int uvc_get_video_ctrl(struct uvc_streaming *stream,
   291		struct uvc_streaming_control *ctrl, int probe, u8 query)
   292	{
   293		u16 size = uvc_video_ctrl_size(stream);
   294		u8 *data __free(kfree) = NULL;
   295		int ret;
   296	
   297		if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) &&
   298				query == UVC_GET_DEF)
   299			return -EIO;
   300	
   301		data = kmalloc(size, GFP_KERNEL);
   302		if (data == NULL)
   303			return -ENOMEM;
   304	
   305		ret = __uvc_query_ctrl(stream->dev, query, 0, stream->intfnum,
   306			probe ? UVC_VS_PROBE_CONTROL : UVC_VS_COMMIT_CONTROL, data,
   307			size, uvc_timeout_param);
   308	
   309		if ((query == UVC_GET_MIN || query == UVC_GET_MAX) && ret == 2) {
   310			/*
   311			 * Some cameras, mostly based on Bison Electronics chipsets,
   312			 * answer a GET_MIN or GET_MAX request with the wCompQuality
   313			 * field only.
   314			 */
   315			uvc_warn_once(stream->dev, UVC_WARN_MINMAX, "UVC non "
   316				"compliance - GET_MIN/MAX(PROBE) incorrectly "
   317				"supported. Enabling workaround.\n");
   318			memset(ctrl, 0, sizeof(*ctrl));
   319			ctrl->wCompQuality = le16_to_cpup((__le16 *)data);
 > 320			return 0
   321		} else if (query == UVC_GET_DEF && probe == 1 && ret != size) {
   322			/*
   323			 * Many cameras don't support the GET_DEF request on their
   324			 * video probe control. Warn once and return, the caller will
   325			 * fall back to GET_CUR.
   326			 */
   327			uvc_warn_once(stream->dev, UVC_WARN_PROBE_DEF, "UVC non "
   328				"compliance - GET_DEF(PROBE) not supported. "
   329				"Enabling workaround.\n");
   330			return -EIO;
   331		} else if (ret != size) {
   332			dev_err(&stream->intf->dev,
   333				"Failed to query (%s) UVC %s control : %d (exp. %u).\n",
   334				uvc_query_name(query), probe ? "probe" : "commit",
   335				ret, size);
   336			return (ret == -EPROTO) ? -EPROTO : -EIO;
   337		}
   338	
   339		ctrl->bmHint = le16_to_cpup((__le16 *)&data[0]);
   340		ctrl->bFormatIndex = data[2];
   341		ctrl->bFrameIndex = data[3];
   342		ctrl->dwFrameInterval = le32_to_cpup((__le32 *)&data[4]);
   343		ctrl->wKeyFrameRate = le16_to_cpup((__le16 *)&data[8]);
   344		ctrl->wPFrameRate = le16_to_cpup((__le16 *)&data[10]);
   345		ctrl->wCompQuality = le16_to_cpup((__le16 *)&data[12]);
   346		ctrl->wCompWindowSize = le16_to_cpup((__le16 *)&data[14]);
   347		ctrl->wDelay = le16_to_cpup((__le16 *)&data[16]);
   348		ctrl->dwMaxVideoFrameSize = get_unaligned_le32(&data[18]);
   349		ctrl->dwMaxPayloadTransferSize = get_unaligned_le32(&data[22]);
   350	
   351		if (size >= 34) {
   352			ctrl->dwClockFrequency = get_unaligned_le32(&data[26]);
   353			ctrl->bmFramingInfo = data[30];
   354			ctrl->bPreferedVersion = data[31];
   355			ctrl->bMinVersion = data[32];
   356			ctrl->bMaxVersion = data[33];
   357		} else {
   358			ctrl->dwClockFrequency = stream->dev->clock_frequency;
   359			ctrl->bmFramingInfo = 0;
   360			ctrl->bPreferedVersion = 0;
   361			ctrl->bMinVersion = 0;
   362			ctrl->bMaxVersion = 0;
   363		}
   364	
   365		/*
   366		 * Some broken devices return null or wrong dwMaxVideoFrameSize and
   367		 * dwMaxPayloadTransferSize fields. Try to get the value from the
   368		 * format and frame descriptors.
   369		 */
   370		uvc_fixup_video_ctrl(stream, ctrl);
   371	
   372		return 0;
   373	}
   374	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] media: uvcvideo: Use scope-based cleanup helper
Posted by Markus Elfring 1 month ago
> Replace the manual kfree() with __free(kfree) annotation for data
> references. This aligns the code with the latest kernel style.

              Align?


…
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -291,7 +291,7 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
>  	struct uvc_streaming_control *ctrl, int probe, u8 query)
>  {
>  	u16 size = uvc_video_ctrl_size(stream);
> -	u8 *data;
> +	u8 *data __free(kfree) = NULL;
>  	int ret;
…

How do you think about to reduce the scopes also for these local variables?


> @@ -317,8 +317,7 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
>  			"supported. Enabling workaround.\n");
>  		memset(ctrl, 0, sizeof(*ctrl));
>  		ctrl->wCompQuality = le16_to_cpup((__le16 *)data);
> -		ret = 0;
> -		goto out;
> +		return 0
>  	} else if (query == UVC_GET_DEF && probe == 1 && ret != size) {
…

Did you accidentally overlook the need for another semicolon here?

Regards,
Markus