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

Chen Changcheng posted 1 patch 1 day, 19 hours ago
drivers/media/usb/uvc/uvc_video.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
[PATCH v2] media: uvcvideo: Use scope-based cleanup helper
Posted by Chen Changcheng 1 day, 19 hours 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 | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 2094e059d7d3..0f524c014d62 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);
@@ -416,11 +410,10 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
 		dev_err(&stream->intf->dev,
 			"Failed to set UVC %s control : %d (exp. %u).\n",
 			probe ? "probe" : "commit", ret, size);
-		ret = -EIO;
+		return -EIO;
 	}
 
-	kfree(data);
-	return ret;
+	return 0;
 }
 
 int uvc_probe_video(struct uvc_streaming *stream,

base-commit: 805f9a061372164d43ddef771d7cd63e3ba6d845
-- 
2.25.1
Re: [PATCH v2] media: uvcvideo: Use scope-based cleanup helper
Posted by Markus Elfring 1 day, 11 hours ago
…
> Replace the manual kfree() with __free(kfree) annotation for data
> references. This aligns the code with the latest kernel style.

              Align?


…
> ---
>  drivers/media/usb/uvc/uvc_video.c | 23 ++++++++---------------
…

Some contributors would appreciate patch version descriptions.
https://lore.kernel.org/all/?q=%22This+looks+like+a+new+version+of+a+previously+submitted+patch%22
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.19-rc4#n310


…
> +++ 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?

Regards,
Markus
Re: [PATCH v2] media: uvcvideo: Use scope-based cleanup helper
Posted by Krzysztof Kozlowski 1 day, 13 hours ago
On 05/01/2026 01:56, Chen Changcheng wrote:
> 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 | 23 ++++++++---------------
>  1 file changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 2094e059d7d3..0f524c014d62 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;


This is an undesired syntax explicitly documented as one to avoid. You
need here proper assignment, not NULL. Please don't use cleanup.h if you
do not intend to follow it because it does not make the code simpler.

Best regards,
Krzysztof
Re: [PATCH v2] uvcvideo: simplify spin_lock using guards
Posted by Chen Changcheng 1 day, 11 hours ago
> This is an undesired syntax explicitly documented as one to avoid. You
> need here proper assignment, not NULL. Please don't use cleanup.h if you
> do not intend to follow it because it does not make the code simpler.

Thank you for your correction and feedback. I made the change initially
because I saw similar patterns using "__free(...) = NULL" in patches from
other driver modules, which led me to overlook the core paradigm of
cleanup.h that requires a "declaration with a meaningful initializer."

Best regards,
Changcheng Chen
Re: [PATCH v2] uvcvideo: simplify spin_lock using guards
Posted by Laurent Pinchart 16 hours ago
On Mon, Jan 05, 2026 at 05:35:12PM +0800, Chen Changcheng wrote:
> > This is an undesired syntax explicitly documented as one to avoid. You
> > need here proper assignment, not NULL. Please don't use cleanup.h if you
> > do not intend to follow it because it does not make the code simpler.
> 
> Thank you for your correction and feedback. I made the change initially
> because I saw similar patterns using "__free(...) = NULL" in patches from
> other driver modules, which led me to overlook the core paradigm of
> cleanup.h that requires a "declaration with a meaningful initializer."

Do you plan to send a v3 ?

-- 
Regards,

Laurent Pinchart
Re: [PATCH v2] media: uvcvideo: Use scope-based cleanup helper
Posted by Chen Changcheng 12 hours ago
> > > This is an undesired syntax explicitly documented as one to avoid. You
> > > need here proper assignment, not NULL. Please don't use cleanup.h if you
> > > do not intend to follow it because it does not make the code simpler.
> >
> > Thank you for your correction and feedback. I made the change initially
> > because I saw similar patterns using "__free(...) = NULL" in patches from
> > other driver modules, which led me to overlook the core paradigm of
> > cleanup.h that requires a "declaration with a meaningful initializer."

> Do you plan to send a v3 ?

No, I don't plan to send a v3. Thank you.

Best regards,
Changcheng Chen