Skip to contentSkip to conversation

drivers: Simplify cleanup paths using __free

PATCH In review Sanjay Chitroda 9 participants 22 messages Mar 10 – Mar 19

Subsystems (9)
Media (V4L/DVB) Staging Arm MediaTek IIO +5 more subsystems
lore
Share & tools
lore

Sanjay Chitroda

author
#0 lore raw

cover drivers: Simplify cleanup paths using __free

From: Sanjay Chitroda <[email protected]>

Hi all,

This patch series replaces manual cleanup and explicit kfree() calls with the __free attribute from <linux/cleanup.h>. This modernizes the memory management style and simplifies common error paths without altering any functional behavior.

The __free attribute provides automatic scope-based cleanup, making resource management clearer and reducing the chances of missing cleanup on early returns.

No functional changes are intended in this series.

Testing:
- Compiled with W=1
- Build-tested on i86_64

Based on:
<linux-v7.0-rc2>

Feel free to share your valuable input in context of the cleanup API.

Thanks,
Sanjay Chitroda

Sanjay Chitroda (7):
staging: greybus: simplify cleanup using __free
iio: ssp_sensors: simplify cleanup using __free
iio: st_sensors: simplify cleanup using __free
media: mediatek: vcodec: simplify cleanup using __free
media: chips-media: coda: simplify cleanup using __free
media: allegro: simplify cleanup using __free
staging: rtl8723bs: simplify cleanup using __free

 drivers/iio/common/ssp_sensors/ssp_spi.c      |  9 +-
 .../iio/common/st_sensors/st_sensors_core.c   |  7 +-
 .../media/platform/allegro-dvt/allegro-core.c | 95 +++++--------------
 .../platform/chips-media/coda/coda-bit.c      |  4 +-
 .../platform/chips-media/coda/coda-jpeg.c     | 39 ++++----
 .../mediatek/vcodec/common/mtk_vcodec_dbgfs.c |  3 +-
 drivers/staging/greybus/camera.c              | 27 ++----
 drivers/staging/greybus/loopback.c            | 35 +++-----
 drivers/staging/greybus/raw.c                 |  6 +-
 .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 13 +--
 drivers/staging/rtl8723bs/hal/sdio_ops.c      | 37 ++------
 11 files changed, 78 insertions(+), 197 deletions(-)
Signature · 1 line
2.34.1
author
#1 lore raw

PATCH 1/7 staging: greybus: simplify cleanup using __free

From: Sanjay Chitroda <[email protected]>

Replace manual cleanup logic with __free attribute from cleanup.h. This removes explicit kfree() calls and simplifies the error handling paths.

No functional change intended for kmalloc().

Signed-off-by Sanjay Chitroda <[email protected]>
---
 drivers/staging/greybus/camera.c   | 27 +++++++----------------
 drivers/staging/greybus/loopback.c | 35 ++++++++++--------------------
 drivers/staging/greybus/raw.c      |  6 ++---
 3 files changed, 22 insertions(+), 46 deletions(-)
drivers/staging/greybus/camera.c +8 -19
index 62b55bb28408..14a603ca2400 100644
@@ -519,8 +519,6 @@ static int gb_camera_configure_streams(struct gb_camera *gcam,
 				       struct gb_camera_stream_config *streams,
 				       struct gb_camera_csi_params *csi_params)
 {
-	struct gb_camera_configure_streams_request *req;
-	struct gb_camera_configure_streams_response *resp;
 	unsigned int nstreams = *num_streams;
 	unsigned int i;
 	size_t req_size;

@@ -533,11 +531,11 @@ static int gb_camera_configure_streams(struct gb_camera *gcam,
 	req_size = sizeof(*req) + nstreams * sizeof(req->config[0]);
 	resp_size = sizeof(*resp) + nstreams * sizeof(resp->config[0]);
 
-	req = kmalloc(req_size, GFP_KERNEL);
-	resp = kmalloc(resp_size, GFP_KERNEL);
+	struct gb_camera_configure_streams_request *req __free(kfree) =
+	    kmalloc(req_size, GFP_KERNEL);
+	struct gb_camera_configure_streams_response *resp __free(kfree) =
+	    kmalloc(resp_size, GFP_KERNEL);
 	if (!req || !resp) {
-		kfree(req);
-		kfree(resp);
 		return -ENOMEM;
 	}
 

@@ -641,8 +639,6 @@ static int gb_camera_configure_streams(struct gb_camera *gcam,
 
 done_skip_pm_put:
 	mutex_unlock(&gcam->mutex);
-	kfree(req);
-	kfree(resp);
 	return ret;
 }
 

@@ -650,7 +646,6 @@ static int gb_camera_capture(struct gb_camera *gcam, u32 request_id,
 			     unsigned int streams, unsigned int num_frames,
 			     size_t settings_size, const void *settings)
 {
-	struct gb_camera_capture_request *req;
 	size_t req_size;
 	int ret;
 

@@ -658,7 +653,8 @@ static int gb_camera_capture(struct gb_camera *gcam, u32 request_id,
 		return -EINVAL;
 
 	req_size = sizeof(*req) + settings_size;
-	req = kmalloc(req_size, GFP_KERNEL);
+	struct gb_camera_capture_request *req __free(kfree) =
+	    kmalloc(req_size, GFP_KERNEL);
 	if (!req)
 		return -ENOMEM;
 

@@ -680,8 +676,6 @@ static int gb_camera_capture(struct gb_camera *gcam, u32 request_id,
 done:
 	mutex_unlock(&gcam->mutex);
 
-	kfree(req);
-
 	return ret;
 }
 

@@ -870,16 +864,15 @@ static ssize_t gb_camera_debugfs_capabilities(struct gb_camera *gcam,
 		&gcam->debugfs.buffers[GB_CAMERA_DEBUGFS_BUFFER_CAPABILITIES];
 	size_t size = 1024;
 	unsigned int i;
-	u8 *caps;
 	int ret;
 
-	caps = kmalloc(size, GFP_KERNEL);
+	u8 *caps __free(kfree) = kmalloc(size, GFP_KERNEL);
 	if (!caps)
 		return -ENOMEM;
 
 	ret = gb_camera_capabilities(gcam, caps, &size);
 	if (ret < 0)
-		goto done;
+		return ret;
 
 	/*
 	 * hex_dump_to_buffer() doesn't return the number of bytes dumped prior

@@ -893,10 +886,6 @@ static ssize_t gb_camera_debugfs_capabilities(struct gb_camera *gcam,
 		buffer->length += sprintf(buffer->data + buffer->length,
 					  "%*ph\n", nbytes, caps + i);
 	}
-
-done:
-	kfree(caps);
-	return ret;
 }
 
 static ssize_t gb_camera_debugfs_configure_streams(struct gb_camera *gcam,
drivers/staging/greybus/loopback.c +12 -23
index aa9c73cb0ae5..ae729f744ac5 100644
@@ -508,10 +508,10 @@ static int gb_loopback_async_operation(struct gb_loopback *gb, int type,
 
 static int gb_loopback_sync_sink(struct gb_loopback *gb, u32 len)
 {
-	struct gb_loopback_transfer_request *request;
 	int retval;
 
-	request = kmalloc(len + sizeof(*request), GFP_KERNEL);
+	struct gb_loopback_transfer_request *request __free(kfree) =
+		kmalloc(len + sizeof(*request), GFP_KERNEL);
 	if (!request)
 		return -ENOMEM;
 

@@ -519,25 +519,24 @@ static int gb_loopback_sync_sink(struct gb_loopback *gb, u32 len)
 	retval = gb_loopback_operation_sync(gb, GB_LOOPBACK_TYPE_SINK,
 					    request, len + sizeof(*request),
 					    NULL, 0);
-	kfree(request);
 	return retval;
 }
 
 static int gb_loopback_sync_transfer(struct gb_loopback *gb, u32 len)
 {
-	struct gb_loopback_transfer_request *request;
-	struct gb_loopback_transfer_response *response;
 	int retval;
 
 	gb->apbridge_latency_ts = 0;
 	gb->gbphy_latency_ts = 0;
 
-	request = kmalloc(len + sizeof(*request), GFP_KERNEL);
+	struct gb_loopback_transfer_request *request __free(kfree) =
+		kmalloc(len + sizeof(*request), GFP_KERNEL);
 	if (!request)
 		return -ENOMEM;
-	response = kmalloc(len + sizeof(*response), GFP_KERNEL);
+
+	struct gb_loopback_transfer_response *response __free(kfree) =
+		kmalloc(len + sizeof(*response), GFP_KERNEL);
 	if (!response) {
-		kfree(request);
 		return -ENOMEM;
 	}
 

@@ -548,7 +547,7 @@ static int gb_loopback_sync_transfer(struct gb_loopback *gb, u32 len)
 					    request, len + sizeof(*request),
 					    response, len + sizeof(*response));
 	if (retval)
-		goto gb_error;
+		return retval;
 
 	if (memcmp(request->data, response->data, len)) {
 		dev_err(&gb->connection->bundle->dev,

@@ -558,10 +557,6 @@ static int gb_loopback_sync_transfer(struct gb_loopback *gb, u32 len)
 	gb->apbridge_latency_ts = (u32)__le32_to_cpu(response->reserved0);
 	gb->gbphy_latency_ts = (u32)__le32_to_cpu(response->reserved1);
 
-gb_error:
-	kfree(request);
-	kfree(response);
-
 	return retval;
 }
 

@@ -573,10 +568,10 @@ static int gb_loopback_sync_ping(struct gb_loopback *gb)
 
 static int gb_loopback_async_sink(struct gb_loopback *gb, u32 len)
 {
-	struct gb_loopback_transfer_request *request;
 	int retval;
 
-	request = kmalloc(len + sizeof(*request), GFP_KERNEL);
+	struct gb_loopback_transfer_request *request __free(kfree) =
+		kmalloc(len + sizeof(*request), GFP_KERNEL);
 	if (!request)
 		return -ENOMEM;
 

@@ -584,7 +579,6 @@ static int gb_loopback_async_sink(struct gb_loopback *gb, u32 len)
 	retval = gb_loopback_async_operation(gb, GB_LOOPBACK_TYPE_SINK,
 					     request, len + sizeof(*request),
 					     0, NULL);
-	kfree(request);
 	return retval;
 }
 

@@ -621,10 +615,10 @@ static int gb_loopback_async_transfer_complete(
 
 static int gb_loopback_async_transfer(struct gb_loopback *gb, u32 len)
 {
-	struct gb_loopback_transfer_request *request;
 	int retval, response_len;
 
-	request = kmalloc(len + sizeof(*request), GFP_KERNEL);
+	struct gb_loopback_transfer_request *request __free(kfree) =
+		kmalloc(len + sizeof(*request), GFP_KERNEL);
 	if (!request)
 		return -ENOMEM;
 

@@ -636,11 +630,6 @@ static int gb_loopback_async_transfer(struct gb_loopback *gb, u32 len)
 					     request, len + sizeof(*request),
 					     len + response_len,
 					     gb_loopback_async_transfer_complete);
-	if (retval)
-		goto gb_error;
-
-gb_error:
-	kfree(request);
 	return retval;
 }
drivers/staging/greybus/raw.c +2 -4
index 3027a2c25bcd..60a754b20432 100644
@@ -126,15 +126,14 @@ static int gb_raw_request_handler(struct gb_operation *op)
 static int gb_raw_send(struct gb_raw *raw, u32 len, const char __user *data)
 {
 	struct gb_connection *connection = raw->connection;
-	struct gb_raw_send_request *request;
 	int retval;
 
-	request = kmalloc(len + sizeof(*request), GFP_KERNEL);
+	struct gb_raw_send_request *request __free(kfree) =
+	    kmalloc(len + sizeof(*request), GFP_KERNEL);
 	if (!request)
 		return -ENOMEM;
 
 	if (copy_from_user(&request->data[0], data, len)) {
-		kfree(request);
 		return -EFAULT;
 	}
 

@@ -144,7 +143,6 @@ static int gb_raw_send(struct gb_raw *raw, u32 len, const char __user *data)
 				   request, len + sizeof(*request),
 				   NULL, 0);
 
-	kfree(request);
 	return retval;
 }
Signature · 1 line
2.34.1
#2 lore raw

Re: [PATCH 1/7] staging: greybus: simplify cleanup using __free

Quoted 4 lines · Sanjay Chitroda

Replace manual cleanup logic with __free attribute from cleanup.h. This removes explicit kfree() calls and simplifies the error handling paths.

No functional change intended for kmalloc().

...

drivers/staging/greybus/camera.c static int gb_camera_configure_streams(struct gb_camera *gcam, · line 538

Quoted 5 lines
drivers/staging/greybus/camera.c +4 -0
+	struct gb_camera_configure_streams_request *req __free(kfree) =
+	    kmalloc(req_size, GFP_KERNEL);
+	struct gb_camera_configure_streams_response *resp __free(kfree) =
+	    kmalloc(resp_size, GFP_KERNEL);
 	if (!req || !resp) {

Now this check should be done in a better way.

drivers/staging/greybus/camera.c static int gb_camera_configure_streams(struct gb_camera *gcam,

Quoted 8 lines
drivers/staging/greybus/camera.c +0 -2
-		kfree(req);
-		kfree(resp);
 		return -ENOMEM;
 	}
 
 
 done_skip_pm_put:
 	mutex_unlock(&gcam->mutex);

To complete this, one may add a prerequisite to use guard()() first.

Quoted 4 lines
drivers/staging/greybus/camera.c +0 -2
-	kfree(req);
-	kfree(resp);
 	return ret;
 }
Signature · 2 lines
With Best Regards,
Andy Shevchenko
#3 lore raw

Re: [PATCH 1/7] staging: greybus: simplify cleanup using __free

Quoted 9 lines · Andy Shevchenko
Quoted 4 lines · Sanjay Chitroda

Replace manual cleanup logic with __free attribute from cleanup.h. This removes explicit kfree() calls and simplifies the error handling paths.

No functional change intended for kmalloc().

...

Quoted 8 lines

drivers/staging/greybus/camera.c static int gb_camera_configure_streams(struct gb_camera *gcam, · line 538

Quoted 5 lines
drivers/staging/greybus/camera.c +4 -0
+	struct gb_camera_configure_streams_request *req __free(kfree) =
+	    kmalloc(req_size, GFP_KERNEL);
+	struct gb_camera_configure_streams_response *resp __free(kfree) =
+	    kmalloc(resp_size, GFP_KERNEL);
 	if (!req || !resp) {

Now this check should be done in a better way.

Yeah, two if statements, right? Drop the curly braces at a minimum.

Quoted 11 lines

drivers/staging/greybus/camera.c static int gb_camera_configure_streams(struct gb_camera *gcam,

Quoted 8 lines
drivers/staging/greybus/camera.c +0 -2
-		kfree(req);
-		kfree(resp);
 		return -ENOMEM;
 	}
 
 
 done_skip_pm_put:
 	mutex_unlock(&gcam->mutex);

To complete this, one may add a prerequisite to use guard()() first.

I don't think we're encouraging people to re-write existing staging code to use cleanup.h magic... It's unclear if I have to review these patches or if they're auto NAKed because we're not doing the conversions.

regards,
dan carpenter

maintainer
#4 lore raw

Re: [PATCH 1/7] staging: greybus: simplify cleanup using __free

Quoted 7 lines · Dan Carpenter
Quoted 2 lines · Andy Shevchenko

To complete this, one may add a prerequisite to use guard()() first.

I don't think we're encouraging people to re-write existing staging code to use cleanup.h magic... It's unclear if I have to review these patches or if they're auto NAKed because we're not doing the conversions.

I've already rejected them, we don't want this type of changes in staging at this point in time.

thanks,

greg k-h

author
#5 lore raw

PATCH 2/7 iio: ssp_sensors: simplify cleanup using __free

In reply to Sanjay Chitroda

From: Sanjay Chitroda <[email protected]>

Replace manual cleanup logic with __free attribute from cleanup.h. This removes explicit kfree() calls and simplifies the error handling paths.

No functional change intended for kmalloc().

Signed-off-by Sanjay Chitroda <[email protected]>
---
 drivers/iio/common/ssp_sensors/ssp_spi.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
drivers/iio/common/ssp_sensors/ssp_spi.c +2 -7
index 6c81c0385fb5..e76ef39c6b7c 100644
@@ -331,7 +331,6 @@ static int ssp_parse_dataframe(struct ssp_data *data, char *dataframe, int len)
 /* threaded irq */
 int ssp_irq_msg(struct ssp_data *data)
 {
-	char *buffer;
 	u8 msg_type;
 	int ret;
 	u16 length, msg_options;

@@ -375,7 +374,7 @@ int ssp_irq_msg(struct ssp_data *data)
 			 * but the slave should not send such ones - it is to
 			 * check but let's handle this
 			 */
-			buffer = kmalloc(length, GFP_KERNEL | GFP_DMA);
+			char *buffer __free(kfree) = kmalloc(length, GFP_KERNEL | GFP_DMA);
 			if (!buffer) {
 				ret = -ENOMEM;
 				goto _unlock;

@@ -386,8 +385,6 @@ int ssp_irq_msg(struct ssp_data *data)
 			if (ret >= 0)
 				ret = -EPROTO;
 
-			kfree(buffer);
-
 			dev_err(SSP_DEV, "No match error %x\n",
 				msg_options);
 

@@ -420,20 +417,18 @@ int ssp_irq_msg(struct ssp_data *data)
 		mutex_unlock(&data->pending_lock);
 		break;
 	case SSP_HUB2AP_WRITE:
-		buffer = kzalloc(length, GFP_KERNEL | GFP_DMA);
+		char *buffer __free(kfree) = kzalloc(length, GFP_KERNEL | GFP_DMA);
 		if (!buffer)
 			return -ENOMEM;
 
 		ret = spi_read(data->spi, buffer, length);
 		if (ret < 0) {
 			dev_err(SSP_DEV, "spi read fail\n");
-			kfree(buffer);
 			break;
 		}
 
 		ret = ssp_parse_dataframe(data, buffer, length);
 
-		kfree(buffer);
 		break;
 
 	default:
Signature · 1 line
2.34.1
#6 lore raw

Re: [PATCH 2/7] iio: ssp_sensors: simplify cleanup using __free

Quoted 5 lines · Sanjay Chitroda

Replace manual cleanup logic with __free attribute from cleanup.h. This removes explicit kfree() calls and simplifies the error handling paths.

No functional change intended for kmalloc().

Why this is a series? You can avoid spamming tons of unrelated people with this by sending patches individually.

...

Quoted 7 lines
drivers/iio/common/ssp_sensors/ssp_spi.c +1 -1
 	case SSP_HUB2AP_WRITE:
-		buffer = kzalloc(length, GFP_KERNEL | GFP_DMA);
+		char *buffer __free(kfree) = kzalloc(length, GFP_KERNEL | GFP_DMA);
 		if (!buffer)
 			return -ENOMEM;
 
 		ret = spi_read(data->spi, buffer, length);

drivers/iio/common/ssp_sensors/ssp_spi.c int ssp_irq_msg(struct ssp_data *data) · line 432

Quoted 10 lines
drivers/iio/common/ssp_sensors/ssp_spi.c +0 -2
 		if (ret < 0) {
 			dev_err(SSP_DEV, "spi read fail\n");
-			kfree(buffer);
 			break;
 		}
 
 		ret = ssp_parse_dataframe(data, buffer, length);
 
-		kfree(buffer);
 		break;

Now you can return directly.

		return ssp_parse_dataframe(data, buffer, length);

But also add a prerequisite to convert to guard()() et alai.

Quoted 1 line

default:

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#7 lore raw

PATCH 3/7 iio: st_sensors: simplify cleanup using __free

In reply to Sanjay Chitroda

From: Sanjay Chitroda <[email protected]>

Replace manual cleanup logic with __free attribute from cleanup.h. This removes explicit kfree() calls and simplifies the error handling paths.

No functional change intended for kmalloc().

Signed-off-by Sanjay Chitroda <[email protected]>
---
 drivers/iio/common/st_sensors/st_sensors_core.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
drivers/iio/common/st_sensors/st_sensors_core.c +2 -5
index dac593be5695..f641b62e692b 100644
@@ -501,14 +501,14 @@ static int st_sensors_read_axis_data(struct iio_dev *indio_dev,
 
 	byte_for_channel = DIV_ROUND_UP(ch->scan_type.realbits +
 					ch->scan_type.shift, 8);
-	outdata = kmalloc(byte_for_channel, GFP_DMA | GFP_KERNEL);
+	u8 *outdata __free(kfree) = kmalloc(byte_for_channel, GFP_DMA | GFP_KERNEL);
 	if (!outdata)
 		return -ENOMEM;
 
 	err = regmap_bulk_read(sdata->regmap, ch->address,
 			       outdata, byte_for_channel);
 	if (err < 0)
-		goto st_sensors_free_memory;
+		return err;
 
 	if (byte_for_channel == 1)
 		*data = (s8)*outdata;

@@ -517,9 +517,6 @@ static int st_sensors_read_axis_data(struct iio_dev *indio_dev,
 	else if (byte_for_channel == 3)
 		*data = (s32)sign_extend32(get_unaligned_le24(outdata), 23);
 
-st_sensors_free_memory:
-	kfree(outdata);
-
 	return err;
 }
Signature · 1 line
2.34.1
reviewer
#8 lore raw

Re: [PATCH 3/7] iio: st_sensors: simplify cleanup using __free

Quoted 17 lines · Sanjay Chitroda

From: Sanjay Chitroda <[email protected]>

Replace manual cleanup logic with __free attribute from cleanup.h. This removes explicit kfree() calls and simplifies the error handling paths.

No functional change intended for kmalloc().

Signed-off-by Sanjay Chitroda <[email protected]>
---
 drivers/iio/common/st_sensors/st_sensors_core.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
drivers/iio/common/st_sensors/st_sensors_core.c +0 -0
index dac593be5695..f641b62e692b 100644

drivers/iio/common/st_sensors/st_sensors_core.c static int st_sensors_read_axis_data(struct iio_dev *indio_dev, · line 504

Quoted 5 lines
drivers/iio/common/st_sensors/st_sensors_core.c +1 -1
@@ -501,14 +501,14 @@ static int st_sensors_read_axis_data(struct iio_dev *indio_dev,
 
 	byte_for_channel = DIV_ROUND_UP(ch->scan_type.realbits +
 					ch->scan_type.shift, 8);
-	outdata = kmalloc(byte_for_channel, GFP_DMA | GFP_KERNEL);
+	u8 *outdata __free(kfree) = kmalloc(byte_for_channel, GFP_DMA | GFP_KERNEL);

Even better would be to not alloc new memory at all.

We could probably reuse buffer_data from struct st_sensor_data for this.

Quoted 12 lines
drivers/iio/common/st_sensors/st_sensors_core.c +1 -1
 	if (!outdata)
 		return -ENOMEM;
 
 	err = regmap_bulk_read(sdata->regmap, ch->address,
 			       outdata, byte_for_channel);
 	if (err < 0)
-		goto st_sensors_free_memory;
+		return err;
 
 	if (byte_for_channel == 1)
 		*data = (s8)*outdata;

drivers/iio/common/st_sensors/st_sensors_core.c static int st_sensors_read_axis_data(struct iio_dev *indio_dev, · line 520

Quoted 7 lines
drivers/iio/common/st_sensors/st_sensors_core.c +0 -3
@@ -517,9 +517,6 @@ static int st_sensors_read_axis_data(struct iio_dev *indio_dev,
 	else if (byte_for_channel == 3)
 		*data = (s32)sign_extend32(get_unaligned_le24(outdata), 23);
 
-st_sensors_free_memory:
-	kfree(outdata);
-
 	return err;

Should be able to change this to return 0; now.

Quoted 2 lines
drivers/iio/common/st_sensors/st_sensors_core.c +0 -0
 }
build bot
#9 lore raw

Re: [PATCH 3/7] iio: st_sensors: simplify cleanup using __free

In reply to Sanjay Chitroda
kernel test robot noticed the following build errors:

Hi Sanjay,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-linus]
[also build test ERROR on jic23-iio/togreg linuxtv-media-pending/master linus/master v7.0-rc3 next-20260311] [cannot apply to staging/staging-testing staging/staging-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Sanjay-Chitroda/staging-greybus-simplify-cleanup-using-__free/20260311-041028
base:   staging/staging-linus
patch link:    https://lore.kernel.org/r/20260310200513.2162018-4-sanjayembedded%40gmail.com
patch subject: [PATCH 3/7] iio: st_sensors: simplify cleanup using __free
config: s390-randconfig-001-20260313 (https://download.01.org/0day-ci/archive/20260313/[email protected]/config)
compiler: s390-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260313/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

drivers/iio/common/st_sensors/st_sensors_core.c: In function 'st_sensors_read_axis_data':

>> drivers/iio/common/st_sensors/st_sensors_core.c:504:6: error: redeclaration of 'outdata' with no linkage
     u8 *outdata __free(kfree) = kmalloc(byte_for_channel, GFP_DMA | GFP_KERNEL);
         ^~~~~~~
   drivers/iio/common/st_sensors/st_sensors_core.c:498:6: note: previous declaration of 'outdata' was here
     u8 *outdata;
         ^~~~~~~

vim +/outdata +504 drivers/iio/common/st_sensors/st_sensors_core.c

493
494 static int st_sensors_read_axis_data(struct iio_dev *indio_dev,
495 struct iio_chan_spec const *ch, int *data)
496 {
497 int err;
498 u8 *outdata;
499 struct st_sensor_data *sdata = iio_priv(indio_dev);
500 unsigned int byte_for_channel;
501
502 byte_for_channel = DIV_ROUND_UP(ch->scan_type.realbits +
503 ch->scan_type.shift, 8);

 > 504		u8 *outdata __free(kfree) = kmalloc(byte_for_channel, GFP_DMA | GFP_KERNEL);

505 if (!outdata)
506 return -ENOMEM;
507
508 err = regmap_bulk_read(sdata->regmap, ch->address,
509 outdata, byte_for_channel);
510 if (err < 0)
511 return err;
512
513 if (byte_for_channel == 1)
514 *data = (s8)*outdata;
515 else if (byte_for_channel == 2)
516 *data = (s16)get_unaligned_le16(outdata);
517 else if (byte_for_channel == 3)
518 *data = (s32)sign_extend32(get_unaligned_le24(outdata), 23);
519
520 return err;
521 }
522

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
build bot
#10 lore raw

Re: [PATCH 3/7] iio: st_sensors: simplify cleanup using __free

In reply to Sanjay Chitroda
kernel test robot noticed the following build errors:

Hi Sanjay,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-linus]
[also build test ERROR on jic23-iio/togreg linuxtv-media-pending/master linus/master v7.0-rc3 next-20260311] [cannot apply to staging/staging-testing staging/staging-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Sanjay-Chitroda/staging-greybus-simplify-cleanup-using-__free/20260311-041028
base:   staging/staging-linus
patch link:    https://lore.kernel.org/r/20260310200513.2162018-4-sanjayembedded%40gmail.com
patch subject: [PATCH 3/7] iio: st_sensors: simplify cleanup using __free
config: riscv-randconfig-001-20260313 (https://download.01.org/0day-ci/archive/20260313/[email protected]/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 7d47b695929cc7f85eeb0f87d0189adc04c1c629)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260313/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All error/warnings (new ones prefixed by >>):

>> drivers/iio/common/st_sensors/st_sensors_core.c:504:14: warning: attribute declaration must precede definition [-Wignored-attributes]
     504 |         u8 *outdata __free(kfree) = kmalloc(byte_for_channel, GFP_DMA | GFP_KERNEL);
         |                     ^
   include/linux/cleanup.h:213:23: note: expanded from macro '__free'
     213 | #define __free(_name)   __cleanup(__free_##_name)
         |                         ^
   include/linux/compiler-clang.h:15:25: note: expanded from macro '__cleanup'
      15 | #define __cleanup(func) __maybe_unused __attribute__((__cleanup__(func)))
         |                         ^
   include/linux/compiler_attributes.h:343:56: note: expanded from macro '__maybe_unused'
     343 | #define __maybe_unused                  __attribute__((__unused__))
         |                                                        ^
   drivers/iio/common/st_sensors/st_sensors_core.c:498:6: note: previous definition is here
     498 |         u8 *outdata;
         |             ^
>> drivers/iio/common/st_sensors/st_sensors_core.c:504:14: warning: attribute declaration must precede definition [-Wignored-attributes]
     504 |         u8 *outdata __free(kfree) = kmalloc(byte_for_channel, GFP_DMA | GFP_KERNEL);
         |                     ^
   include/linux/cleanup.h:213:23: note: expanded from macro '__free'
     213 | #define __free(_name)   __cleanup(__free_##_name)
         |                         ^
   include/linux/compiler-clang.h:15:55: note: expanded from macro '__cleanup'
      15 | #define __cleanup(func) __maybe_unused __attribute__((__cleanup__(func)))
         |                                                       ^
   drivers/iio/common/st_sensors/st_sensors_core.c:498:6: note: previous definition is here
     498 |         u8 *outdata;
         |             ^
>> drivers/iio/common/st_sensors/st_sensors_core.c:504:6: error: redefinition of 'outdata'
     504 |         u8 *outdata __free(kfree) = kmalloc(byte_for_channel, GFP_DMA | GFP_KERNEL);
         |             ^
   drivers/iio/common/st_sensors/st_sensors_core.c:498:6: note: previous definition is here
     498 |         u8 *outdata;
         |             ^

2 warnings and 1 error generated.

vim +/outdata +504 drivers/iio/common/st_sensors/st_sensors_core.c

493
494 static int st_sensors_read_axis_data(struct iio_dev *indio_dev,
495 struct iio_chan_spec const *ch, int *data)
496 {
497 int err;

 > 498		u8 *outdata;
   499		struct st_sensor_data *sdata = iio_priv(indio_dev);
   500		unsigned int byte_for_channel;
   501	
   502		byte_for_channel = DIV_ROUND_UP(ch->scan_type.realbits +
   503						ch->scan_type.shift, 8);
 > 504		u8 *outdata __free(kfree) = kmalloc(byte_for_channel, GFP_DMA | GFP_KERNEL);

505 if (!outdata)
506 return -ENOMEM;
507
508 err = regmap_bulk_read(sdata->regmap, ch->address,
509 outdata, byte_for_channel);
510 if (err < 0)
511 return err;
512
513 if (byte_for_channel == 1)
514 *data = (s8)*outdata;
515 else if (byte_for_channel == 2)
516 *data = (s16)get_unaligned_le16(outdata);
517 else if (byte_for_channel == 3)
518 *data = (s32)sign_extend32(get_unaligned_le24(outdata), 23);
519
520 return err;
521 }
522

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
author
#11 lore raw

PATCH 4/7 media: mediatek: vcodec: simplify cleanup using __free

In reply to Sanjay Chitroda

From: Sanjay Chitroda <[email protected]>

Replace manual cleanup logic with __free attribute from cleanup.h. This removes explicit kfree() calls and simplifies the error handling paths.

No functional change intended for kmalloc().

Signed-off-by Sanjay Chitroda <[email protected]>
---
 .../media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c   | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c +1 -2
index 2da11521fc7b..3184939f793a 100644
@@ -96,7 +96,7 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf,
 	int total_len = 200 * (dbgfs->inst_count == 0 ? 1 : dbgfs->inst_count);
 	int used_len = 0, curr_len, ret;
 	bool dbgfs_index[MTK_VDEC_DBGFS_MAX] = {0};
-	char *buf = kmalloc(total_len, GFP_KERNEL);
+	char *buf __free(kfree) = kmalloc(total_len, GFP_KERNEL);
 
 	if (!buf)
 		return -ENOMEM;

@@ -134,7 +134,6 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf,
 	mutex_unlock(&dbgfs->dbgfs_lock);
 read_buffer:
 	ret = simple_read_from_buffer(ubuf, count, ppos, buf, used_len);
-	kfree(buf);
 	return ret;
 }
Signature · 1 line
2.34.1
#12 lore raw

Re: [PATCH 4/7] media: mediatek: vcodec: simplify cleanup using __free

Hi,

Le mercredi 11 mars 2026 à 01:35 +0530, Sanjay Chitroda a écrit :

Quoted 6 lines

From: Sanjay Chitroda <[email protected]>

Replace manual cleanup logic with __free attribute from cleanup.h. This removes explicit kfree() calls and simplifies the error handling paths.

No functional change intended for kmalloc().

While I like auto cleanup, I think consistency is key. kmalloc is a tiny little dot in the sea here. Most of our leaks are in probe() error handling. In v4l2, you find a log of init() with matching releas() call, which get constantly forgotton. My suggestion would be to focus on one driver at the time, not kmalloc across the kernel, and try and "port" these driver to consistently use the cleanup function. This should also come with usage of quard() as its the same objective.

You goal should be to return at any point in the function without risking of leaving lock/spinlock held or leaving memory.

Marking as change requested the codec releated patches in this series.

Nicolas

Quoted 31 lines
Signed-off-by Sanjay Chitroda <[email protected]>

---
 .../media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c   | 3 +--

 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c +0 -0
index 2da11521fc7b..3184939f793a 100644
@@ -96,7 +96,7 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char
 __user *ubuf,

  int total_len = 200 * (dbgfs->inst_count == 0 ? 1 : dbgfs-

Quoted 1 line

inst_count);

  int used_len = 0, curr_len, ret; bool dbgfs_index[MTK_VDEC_DBGFS_MAX] = {0};

drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c +1 -1
-	char *buf = kmalloc(total_len, GFP_KERNEL);
+	char *buf __free(kfree) = kmalloc(total_len, GFP_KERNEL);


  if (!buf)
  return -ENOMEM;

drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c +0 -0
@@ -134,7 +134,6 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char
 __user *ubuf,
  	mutex_unlock(&dbgfs->dbgfs_lock);

 read_buffer:
  ret = simple_read_from_buffer(ubuf, count, ppos, buf, used_len);

drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_dbgfs.c +0 -1
-	kfree(buf);

  return ret;
 }

author
#13 lore raw

PATCH 5/7 media: chips-media: coda: simplify cleanup using __free

In reply to Sanjay Chitroda

From: Sanjay Chitroda <[email protected]>

Replace manual cleanup logic with __free attribute from cleanup.h. This removes explicit kfree() calls and simplifies the error handling paths.

No functional change intended for kmalloc()/kzalloc_obj().

Signed-off-by Sanjay Chitroda <[email protected]>
---
 .../platform/chips-media/coda/coda-bit.c      |  4 +-
 .../platform/chips-media/coda/coda-jpeg.c     | 39 +++++++------------
 2 files changed, 16 insertions(+), 27 deletions(-)
drivers/media/platform/chips-media/coda/coda-bit.c +1 -3
index b0559303c40f..958e245d0698 100644
@@ -183,19 +183,17 @@ static void coda_kfifo_sync_to_device_write(struct coda_ctx *ctx)
 
 static int coda_h264_bitstream_pad(struct coda_ctx *ctx, u32 size)
 {
-	unsigned char *buf;
 	u32 n;
 
 	if (size < 6)
 		size = 6;
 
-	buf = kmalloc(size, GFP_KERNEL);
+	unsigned char *buf __free(kfree) = kmalloc(size, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
 	coda_h264_filler_nal(size, buf);
 	n = kfifo_in(&ctx->bitstream_fifo, buf, size);
-	kfree(buf);
 
 	return (n < size) ? -ENOSPC : 0;
 }
drivers/media/platform/chips-media/coda/coda-jpeg.c +15 -24
index 835225383aa1..11f2800014e5 100644
@@ -584,16 +584,15 @@ static int coda9_jpeg_gen_enc_huff_tab(struct coda_ctx *ctx, int tab_num,
 {
 	int i, j, k, lastk, si, code, maxsymbol;
 	const u8 *bits, *huffval;
-	struct {
-		int size[256];
-		int code[256];
-	} *huff;
 	static const unsigned char *huff_tabs[4] = {
 		luma_dc, luma_ac, chroma_dc, chroma_ac,
 	};
 	int ret = -EINVAL;
 
-	huff = kzalloc_obj(*huff);
+	struct {
+		int size[256];
+		int code[256];
+	} *huff __free(kfree) = kzalloc_obj(*huff);
 	if (!huff)
 		return -ENOMEM;
 

@@ -607,7 +606,7 @@ static int coda9_jpeg_gen_enc_huff_tab(struct coda_ctx *ctx, int tab_num,
 	for (i = 1; i <= 16; i++) {
 		j = bits[i - 1];
 		if (k + j > maxsymbol)
-			goto out;
+			return ret;
 		while (j--)
 			huff->size[k++] = i;
 	}

@@ -623,7 +622,7 @@ static int coda9_jpeg_gen_enc_huff_tab(struct coda_ctx *ctx, int tab_num,
 			code++;
 		}
 		if (code >= (1 << si))
-			goto out;
+			return ret;
 		code <<= 1;
 		si++;
 	}

@@ -632,15 +631,12 @@ static int coda9_jpeg_gen_enc_huff_tab(struct coda_ctx *ctx, int tab_num,
 	for (k = 0; k < lastk; k++) {
 		i = huffval[k];
 		if (i >= maxsymbol || ehufsi[i])
-			goto out;
+			return ret;
 		ehufco[i] = huff->code[k];
 		ehufsi[i] = huff->size[k];
 	}
 
-	ret = 0;
-out:
-	kfree(huff);
-	return ret;
+	return 0;
 }
 
 #define DC_TABLE_INDEX0		    0

@@ -715,15 +711,14 @@ static int coda9_jpeg_gen_dec_huff_tab(struct coda_ctx *ctx, int tab_num)
 
 static int coda9_jpeg_load_huff_tab(struct coda_ctx *ctx)
 {
-	struct {
-		int size[4][256];
-		int code[4][256];
-	} *huff;
 	u32 *huff_data;
 	int i, j;
 	int ret;
 
-	huff = kzalloc_obj(*huff);
+	struct {
+		int size[4][256];
+		int code[4][256];
+	} *huff __free(kfree) = kzalloc_obj(*huff);
 	if (!huff)
 		return -ENOMEM;
 

@@ -732,7 +727,7 @@ static int coda9_jpeg_load_huff_tab(struct coda_ctx *ctx)
 		ret = coda9_jpeg_gen_enc_huff_tab(ctx, i, huff->size[i],
 						  huff->code[i]);
 		if (ret)
-			goto out;
+			return ret;
 	}
 
 	if (!ctx->params.jpeg_huff_data) {

@@ -740,8 +735,7 @@ static int coda9_jpeg_load_huff_tab(struct coda_ctx *ctx)
 			kzalloc(sizeof(u32) * CODA9_JPEG_ENC_HUFF_DATA_SIZE,
 				GFP_KERNEL);
 		if (!ctx->params.jpeg_huff_data) {
-			ret = -ENOMEM;
-			goto out;
+			return -ENOMEM;
 		}
 	}
 	huff_data = ctx->params.jpeg_huff_data;

@@ -765,10 +759,7 @@ static int coda9_jpeg_load_huff_tab(struct coda_ctx *ctx)
 		}
 	}
 
-	ret = 0;
-out:
-	kfree(huff);
-	return ret;
+	return 0;
 }
 
 static void coda9_jpeg_write_huff_tab(struct coda_ctx *ctx)
Signature · 1 line
2.34.1
author
#14 lore raw

PATCH 6/7 media: allegro: simplify cleanup using __free

In reply to Sanjay Chitroda

From: Sanjay Chitroda <[email protected]>

Replace manual cleanup logic with __free attribute from cleanup.h. This removes explicit kfree() calls and simplifies the error handling paths. Remove unused variable e.g. ‘size’ after usage of cleanup API.

No functional change intended for kernel memory allocation.

Signed-off-by Sanjay Chitroda <[email protected]>
---
 .../media/platform/allegro-dvt/allegro-core.c | 95 +++++--------------
 1 file changed, 24 insertions(+), 71 deletions(-)
drivers/media/platform/allegro-dvt/allegro-core.c +24 -71
index eac3bc9af990..197a368fcb57 100644
@@ -934,25 +934,20 @@ static int allegro_mbox_send(struct allegro_mbox *mbox, void *msg)
 {
 	struct allegro_dev *dev = mbox->dev;
 	ssize_t size;
-	int err;
-	u32 *tmp;
+	int err = 0;
 
-	tmp = kzalloc(mbox->size, GFP_KERNEL);
-	if (!tmp) {
-		err = -ENOMEM;
-		goto out;
-	}
+	u32 *tmp __free(kfree) = kzalloc(mbox->size, GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
 
 	size = allegro_encode_mail(tmp, msg);
 
 	err = allegro_mbox_write(mbox, tmp, size);
-	kfree(tmp);
 	if (err)
-		goto out;
+		return err;
 
 	allegro_mcu_interrupt(dev);
 
-out:
 	return err;
 }
 

@@ -963,36 +958,29 @@ static int allegro_mbox_send(struct allegro_mbox *mbox, void *msg)
 static int allegro_mbox_notify(struct allegro_mbox *mbox)
 {
 	struct allegro_dev *dev = mbox->dev;
-	union mcu_msg_response *msg;
-	u32 *tmp;
-	int err;
+	int err = 0;
 
-	msg = kmalloc_obj(*msg);
+	union mcu_msg_response *msg __free(kfree) = kmalloc_obj(*msg);
 	if (!msg)
 		return -ENOMEM;
 
 	msg->header.version = dev->fw_info->mailbox_version;
 
-	tmp = kmalloc(mbox->size, GFP_KERNEL);
+	u32 *tmp __free(kfree) = kmalloc(mbox->size, GFP_KERNEL);
 	if (!tmp) {
-		err = -ENOMEM;
-		goto out;
+		return -ENOMEM;
 	}
 
 	err = allegro_mbox_read(mbox, tmp, mbox->size);
 	if (err < 0)
-		goto out;
+		return err;
 
 	err = allegro_decode_mail(msg, tmp);
 	if (err)
-		goto out;
+		return err;
 
 	allegro_handle_message(dev, msg);
 
-out:
-	kfree(tmp);
-	kfree(msg);
-
 	return err;
 }
 

@@ -1480,13 +1468,11 @@ static int allegro_mcu_push_buffer_internal(struct allegro_channel *channel,
 					    enum mcu_msg_type type)
 {
 	struct allegro_dev *dev = channel->dev;
-	struct mcu_msg_push_buffers_internal *msg;
 	struct mcu_msg_push_buffers_internal_buffer *buffer;
 	unsigned int num_buffers = 0;
 	size_t size;
 	struct allegro_buffer *al_buffer;
 	struct list_head *list;
-	int err;
 
 	switch (type) {
 	case MCU_MSG_TYPE_PUSH_BUFFER_REFERENCE:

@@ -1501,9 +1487,9 @@ static int allegro_mcu_push_buffer_internal(struct allegro_channel *channel,
 
 	list_for_each_entry(al_buffer, list, head)
 		num_buffers++;
-	size = struct_size(msg, buffer, num_buffers);
+	size = struct_size((struct mcu_msg_push_buffers_internal *)NULL, buffer, num_buffers);
 
-	msg = kmalloc(size, GFP_KERNEL);
+	struct mcu_msg_push_buffers_internal *msg __free(kfree) = kmalloc(size, GFP_KERNEL);
 	if (!msg)
 		return -ENOMEM;
 

@@ -1521,10 +1507,7 @@ static int allegro_mcu_push_buffer_internal(struct allegro_channel *channel,
 		buffer++;
 	}
 
-	err = allegro_mbox_send(dev->mbox_command, msg);
-
-	kfree(msg);
-	return err;
+	return allegro_mbox_send(dev->mbox_command, msg);
 }
 
 static int allegro_mcu_push_buffer_intermediate(struct allegro_channel *channel)

@@ -1621,8 +1604,6 @@ static ssize_t allegro_h264_write_sps(struct allegro_channel *channel,
 				      void *dest, size_t n)
 {
 	struct allegro_dev *dev = channel->dev;
-	struct nal_h264_sps *sps;
-	ssize_t size;
 	unsigned int size_mb = SIZE_MACROBLOCK;
 	/* Calculation of crop units in Rec. ITU-T H.264 (04/2017) p. 76 */
 	unsigned int crop_unit_x = 2;

@@ -1632,7 +1613,7 @@ static ssize_t allegro_h264_write_sps(struct allegro_channel *channel,
 	unsigned int cpb_size;
 	unsigned int cpb_size_scale;
 
-	sps = kzalloc_obj(*sps);
+	struct nal_h264_sps *sps __free(kfree) = kzalloc_obj(*sps);
 	if (!sps)
 		return -ENOMEM;
 

@@ -1715,21 +1696,15 @@ static ssize_t allegro_h264_write_sps(struct allegro_channel *channel,
 	sps->vui.pic_struct_present_flag = 1;
 	sps->vui.bitstream_restriction_flag = 0;
 
-	size = nal_h264_write_sps(&dev->plat_dev->dev, dest, n, sps);
-
-	kfree(sps);
-
-	return size;
+	return nal_h264_write_sps(&dev->plat_dev->dev, dest, n, sps);
 }
 
 static ssize_t allegro_h264_write_pps(struct allegro_channel *channel,
 				      void *dest, size_t n)
 {
 	struct allegro_dev *dev = channel->dev;
-	struct nal_h264_pps *pps;
-	ssize_t size;
 
-	pps = kzalloc_obj(*pps);
+	struct nal_h264_pps *pps __free(kfree) = kzalloc_obj(*pps);
 	if (!pps)
 		return -ENOMEM;
 

@@ -1752,11 +1727,7 @@ static ssize_t allegro_h264_write_pps(struct allegro_channel *channel,
 	pps->pic_scaling_matrix_present_flag = 0;
 	pps->second_chroma_qp_index_offset = 0;
 
-	size = nal_h264_write_pps(&dev->plat_dev->dev, dest, n, pps);
-
-	kfree(pps);
-
-	return size;
+	return nal_h264_write_pps(&dev->plat_dev->dev, dest, n, pps);
 }
 
 static void allegro_channel_eos_event(struct allegro_channel *channel)

@@ -1772,15 +1743,13 @@ static ssize_t allegro_hevc_write_vps(struct allegro_channel *channel,
 				      void *dest, size_t n)
 {
 	struct allegro_dev *dev = channel->dev;
-	struct nal_hevc_vps *vps;
 	struct nal_hevc_profile_tier_level *ptl;
-	ssize_t size;
 	unsigned int num_ref_frames = channel->num_ref_idx_l0;
 	s32 profile = v4l2_ctrl_g_ctrl(channel->mpeg_video_hevc_profile);
 	s32 level = v4l2_ctrl_g_ctrl(channel->mpeg_video_hevc_level);
 	s32 tier = v4l2_ctrl_g_ctrl(channel->mpeg_video_hevc_tier);
 
-	vps = kzalloc_obj(*vps);
+	struct nal_hevc_vps *vps __free(kfree) = kzalloc_obj(*vps);
 	if (!vps)
 		return -ENOMEM;
 

@@ -1800,29 +1769,23 @@ static ssize_t allegro_hevc_write_vps(struct allegro_channel *channel,
 	vps->max_dec_pic_buffering_minus1[0] = num_ref_frames;
 	vps->max_num_reorder_pics[0] = num_ref_frames;
 
-	size = nal_hevc_write_vps(&dev->plat_dev->dev, dest, n, vps);
-
-	kfree(vps);
-
-	return size;
+	return nal_hevc_write_vps(&dev->plat_dev->dev, dest, n, vps);
 }
 
 static ssize_t allegro_hevc_write_sps(struct allegro_channel *channel,
 				      void *dest, size_t n)
 {
 	struct allegro_dev *dev = channel->dev;
-	struct nal_hevc_sps *sps;
 	struct nal_hevc_profile_tier_level *ptl;
 	struct nal_hevc_vui_parameters *vui;
 	struct nal_hevc_hrd_parameters *hrd;
-	ssize_t size;
 	unsigned int cpb_size;
 	unsigned int num_ref_frames = channel->num_ref_idx_l0;
 	s32 profile = v4l2_ctrl_g_ctrl(channel->mpeg_video_hevc_profile);
 	s32 level = v4l2_ctrl_g_ctrl(channel->mpeg_video_hevc_level);
 	s32 tier = v4l2_ctrl_g_ctrl(channel->mpeg_video_hevc_tier);
 
-	sps = kzalloc_obj(*sps);
+	struct nal_hevc_sps *sps __free(kfree) = kzalloc_obj(*sps);
 	if (!sps)
 		return -ENOMEM;
 

@@ -1913,11 +1876,7 @@ static ssize_t allegro_hevc_write_sps(struct allegro_channel *channel,
 
 	hrd->vcl_hrd[0].cbr_flag[0] = !v4l2_ctrl_g_ctrl(channel->mpeg_video_frame_rc_enable);
 
-	size = nal_hevc_write_sps(&dev->plat_dev->dev, dest, n, sps);
-
-	kfree(sps);
-
-	return size;
+	return nal_hevc_write_sps(&dev->plat_dev->dev, dest, n, sps);
 }
 
 static ssize_t allegro_hevc_write_pps(struct allegro_channel *channel,

@@ -1925,11 +1884,9 @@ static ssize_t allegro_hevc_write_pps(struct allegro_channel *channel,
 				      void *dest, size_t n)
 {
 	struct allegro_dev *dev = channel->dev;
-	struct nal_hevc_pps *pps;
-	ssize_t size;
 	int i;
 
-	pps = kzalloc_obj(*pps);
+	struct nal_hevc_pps *pps = kzalloc_obj(*pps);
 	if (!pps)
 		return -ENOMEM;
 

@@ -1960,11 +1917,7 @@ static ssize_t allegro_hevc_write_pps(struct allegro_channel *channel,
 
 	pps->lists_modification_present_flag = channel->enable_reordering;
 
-	size = nal_hevc_write_pps(&dev->plat_dev->dev, dest, n, pps);
-
-	kfree(pps);
-
-	return size;
+	return nal_hevc_write_pps(&dev->plat_dev->dev, dest, n, pps);
 }
 
 static u64 allegro_put_buffer(struct allegro_channel *channel,
Signature · 1 line
2.34.1
author
#15 lore raw

PATCH 7/7 staging: rtl8723bs: simplify cleanup using __free

In reply to Sanjay Chitroda

From: Sanjay Chitroda <[email protected]>

Replace manual cleanup logic with __free attribute from cleanup.h. This removes explicit kfree() calls and simplifies the error handling paths.

No functional change intended for kmalloc().

Signed-off-by Sanjay Chitroda <[email protected]>
---
 .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 13 ++-----
 drivers/staging/rtl8723bs/hal/sdio_ops.c      | 37 ++++---------------
 2 files changed, 11 insertions(+), 39 deletions(-)
drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c +3 -10
index 8d259820f103..2badf7d1aec4 100644
@@ -627,7 +627,6 @@ static void hal_ReadEFuse_WiFi(
 	u8 *pbuf
 )
 {
-	u8 *efuseTbl = NULL;
 	u16 eFuse_Addr = 0;
 	u8 offset, wden;
 	u8 efuseHeader, efuseExtHdr, efuseData;

@@ -640,7 +639,7 @@ static void hal_ReadEFuse_WiFi(
 	if ((_offset + _size_byte) > EFUSE_MAX_MAP_LEN)
 		return;
 
-	efuseTbl = kmalloc(EFUSE_MAX_MAP_LEN, GFP_ATOMIC);
+	u8 *efuseTbl __free(kfree) = kmalloc(EFUSE_MAX_MAP_LEN, GFP_ATOMIC);
 	if (!efuseTbl)
 		return;
 

@@ -702,8 +701,6 @@ static void hal_ReadEFuse_WiFi(
 
 	rtw_hal_set_hwreg(padapter, HW_VAR_EFUSE_BYTES, (u8 *)&used);
 	rtw_hal_set_hwreg(padapter, HW_VAR_EFUSE_USAGE, (u8 *)&efuse_usage);
-
-	kfree(efuseTbl);
 }
 
 static void hal_ReadEFuse_BT(

@@ -713,7 +710,6 @@ static void hal_ReadEFuse_BT(
 	u8 *pbuf
 )
 {
-	u8 *efuseTbl;
 	u8 bank;
 	u16 eFuse_Addr;
 	u8 efuseHeader, efuseExtHdr, efuseData;

@@ -728,7 +724,7 @@ static void hal_ReadEFuse_BT(
 	if ((_offset + _size_byte) > EFUSE_BT_MAP_LEN)
 		return;
 
-	efuseTbl = kmalloc(EFUSE_BT_MAP_LEN, GFP_ATOMIC);
+	u8 *efuseTbl __free(kfree) = kmalloc(EFUSE_BT_MAP_LEN, GFP_ATOMIC);
 	if (!efuseTbl)
 		return;
 

@@ -739,7 +735,7 @@ static void hal_ReadEFuse_BT(
 
 	for (bank = 1; bank < 3; bank++) { /*  8723b Max bake 0~2 */
 		if (hal_EfuseSwitchToBank(padapter, bank) == false)
-			goto exit;
+			return;
 
 		eFuse_Addr = 0;
 

@@ -804,9 +800,6 @@ static void hal_ReadEFuse_BT(
 
 	rtw_hal_set_hwreg(padapter, HW_VAR_EFUSE_BT_BYTES, (u8 *)&used);
 	rtw_hal_set_hwreg(padapter, HW_VAR_EFUSE_BT_USAGE, (u8 *)&efuse_usage);
-
-exit:
-	kfree(efuseTbl);
 }
 
 void Hal_ReadEFuse(
drivers/staging/rtl8723bs/hal/sdio_ops.c +8 -29
index c9cb20c61a2b..303139a75551 100644
@@ -179,9 +179,7 @@ static u32 sdio_read32(struct intf_hdl *intfhdl, u32 addr)
 	if (shift == 0) {
 		val = sd_read32(intfhdl, ftaddr, NULL);
 	} else {
-		u8 *tmpbuf;
-
-		tmpbuf = kmalloc(8, GFP_ATOMIC);
+		u8 *tmpbuf __free(kfree) = kmalloc(8, GFP_ATOMIC);
 		if (!tmpbuf)
 			return SDIO_ERR_VAL32;
 

@@ -189,8 +187,6 @@ static u32 sdio_read32(struct intf_hdl *intfhdl, u32 addr)
 		sd_read(intfhdl, ftaddr, 8, tmpbuf);
 		memcpy(&le_tmp, tmpbuf + shift, 4);
 		val = le32_to_cpu(le_tmp);
-
-		kfree(tmpbuf);
 	}
 	return val;
 }

@@ -223,19 +219,17 @@ static s32 sdio_readN(struct intf_hdl *intfhdl, u32 addr, u32 cnt, u8 *buf)
 	if (shift == 0) {
 		err = sd_read(intfhdl, ftaddr, cnt, buf);
 	} else {
-		u8 *tmpbuf;
 		u32 n;
 
 		ftaddr &= ~(u16)0x3;
 		n = cnt + shift;
-		tmpbuf = kmalloc(n, GFP_ATOMIC);
+		u8 *tmpbuf __free(kfree) = kmalloc(n, GFP_ATOMIC);
 		if (!tmpbuf)
 			return -ENOMEM;
 
 		err = sd_read(intfhdl, ftaddr, n, tmpbuf);
 		if (!err)
 			memcpy(buf, tmpbuf + shift, cnt);
-		kfree(tmpbuf);
 	}
 	return err;
 }

@@ -326,22 +320,18 @@ static s32 sdio_writeN(struct intf_hdl *intfhdl, u32 addr, u32 cnt, u8 *buf)
 	if (shift == 0) {
 		err = sd_write(intfhdl, ftaddr, cnt, buf);
 	} else {
-		u8 *tmpbuf;
 		u32 n;
 
 		ftaddr &= ~(u16)0x3;
 		n = cnt + shift;
-		tmpbuf = kmalloc(n, GFP_ATOMIC);
+		u8 *tmpbuf __free(kfree) = kmalloc(n, GFP_ATOMIC);
 		if (!tmpbuf)
 			return -ENOMEM;
 		err = sd_read(intfhdl, ftaddr, 4, tmpbuf);
-		if (err) {
-			kfree(tmpbuf);
+		if (err)
 			return err;
-		}
 		memcpy(tmpbuf + shift, buf, cnt);
 		err = sd_write(intfhdl, ftaddr, n, tmpbuf);
-		kfree(tmpbuf);
 	}
 	return err;
 }

@@ -491,7 +481,6 @@ static s32 _sdio_local_read(
 	struct intf_hdl *intfhdl;
 	u8 mac_pwr_ctrl_on;
 	s32 err;
-	u8 *tmpbuf;
 	u32 n;
 
 	intfhdl = &adapter->iopriv.intf;

@@ -503,7 +492,7 @@ static s32 _sdio_local_read(
 		return _sd_cmd52_read(intfhdl, addr, cnt, buf);
 
 	n = round_up(cnt, 4);
-	tmpbuf = kmalloc(n, GFP_ATOMIC);
+	u8 *tmpbuf __free(kfree) = kmalloc(n, GFP_ATOMIC);
 	if (!tmpbuf)
 		return -ENOMEM;
 

@@ -511,8 +500,6 @@ static s32 _sdio_local_read(
 	if (!err)
 		memcpy(buf, tmpbuf, cnt);
 
-	kfree(tmpbuf);
-
 	return err;
 }
 

@@ -529,7 +516,6 @@ s32 sdio_local_read(
 	struct intf_hdl *intfhdl;
 	u8 mac_pwr_ctrl_on;
 	s32 err;
-	u8 *tmpbuf;
 	u32 n;
 
 	intfhdl = &adapter->iopriv.intf;

@@ -544,7 +530,7 @@ s32 sdio_local_read(
 		return sd_cmd52_read(intfhdl, addr, cnt, buf);
 
 	n = round_up(cnt, 4);
-	tmpbuf = kmalloc(n, GFP_ATOMIC);
+	u8 *tmpbuf __free(kfree) = kmalloc(n, GFP_ATOMIC);
 	if (!tmpbuf)
 		return -ENOMEM;
 

@@ -552,8 +538,6 @@ s32 sdio_local_read(
 	if (!err)
 		memcpy(buf, tmpbuf, cnt);
 
-	kfree(tmpbuf);
-
 	return err;
 }
 

@@ -570,7 +554,6 @@ s32 sdio_local_write(
 	struct intf_hdl *intfhdl;
 	u8 mac_pwr_ctrl_on;
 	s32 err;
-	u8 *tmpbuf;
 
 	intfhdl = &adapter->iopriv.intf;
 

@@ -583,7 +566,7 @@ s32 sdio_local_write(
 	)
 		return sd_cmd52_write(intfhdl, addr, cnt, buf);
 
-	tmpbuf = kmalloc(cnt, GFP_ATOMIC);
+	u8 *tmpbuf __free(kfree) = kmalloc(cnt, GFP_ATOMIC);
 	if (!tmpbuf)
 		return -ENOMEM;
 

@@ -591,8 +574,6 @@ s32 sdio_local_write(
 
 	err = sd_write(intfhdl, addr, cnt, tmpbuf);
 
-	kfree(tmpbuf);
-
 	return err;
 }
 

@@ -880,16 +861,14 @@ void sd_int_dpc(struct adapter *adapter)
 	}
 
 	if (hal->sdio_hisr & SDIO_HISR_TXERR) {
-		u8 *status;
 		u32 addr;
 
-		status = kmalloc(4, GFP_ATOMIC);
+		u8 *status  __free(kfree) = kmalloc(4, GFP_ATOMIC);
 		if (status) {
 			addr = REG_TXDMA_STATUS;
 			hal_sdio_get_cmd_addr_8723b(adapter, WLAN_IOREG_DEVICE_ID, addr, &addr);
 			_sd_read(intfhdl, addr, 4, status);
 			_sd_write(intfhdl, addr, 4, status);
-			kfree(status);
 		}
 	}
Signature · 1 line
2.34.1
#16 lore raw

Re: [PATCH 7/7] staging: rtl8723bs: simplify cleanup using __free

Quoted 15 lines · Sanjay Chitroda

From: Sanjay Chitroda <[email protected]>

Replace manual cleanup logic with __free attribute from cleanup.h. This removes explicit kfree() calls and simplifies the error handling paths.

No functional change intended for kmalloc().

Signed-off-by Sanjay Chitroda <[email protected]>
---
 .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 13 ++-----
 drivers/staging/rtl8723bs/hal/sdio_ops.c      | 37 ++++---------------
 2 files changed, 11 insertions(+), 39 deletions(-)
drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c +0 -0
index 8d259820f103..2badf7d1aec4 100644

drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c

Quoted 1 line
drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c +0 -1
--- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c

rtl8723bs is a networking device.

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

says:

Low level cleanup constructs (such as __free()) can be used when building APIs and helpers, especially scoped iterators. However, direct use of __free() within networking core and drivers is discouraged.

Please drop this patch.

You might also want to check other subsystems and see if they have similar policies for these magic operators.

Andrew

maintainer
#17 lore raw

Re: [PATCH 7/7] staging: rtl8723bs: simplify cleanup using __free

In reply to Sanjay Chitroda
Quoted 5 lines · Sanjay Chitroda

From: Sanjay Chitroda <[email protected]>

Replace manual cleanup logic with __free attribute from cleanup.h. This removes explicit kfree() calls and simplifies the error handling paths.

Quoted 7 lines

No functional change intended for kmalloc().

Signed-off-by Sanjay Chitroda <[email protected]>
---
 .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 13 ++-----
 drivers/staging/rtl8723bs/hal/sdio_ops.c      | 37 ++++---------------
 2 files changed, 11 insertions(+), 39 deletions(-)

As has been stated many times in the past, please do this for new code, but not for existing code unless you are fixing a bug at the same time, as the churn is not worth it (especially if you do not have the hardware to test the changes with.)

thanks,

greg k-h

#18 lore raw
In reply to Sanjay Chitroda
Quoted 15 lines · Sanjay Chitroda

From: Sanjay Chitroda <[email protected]>

Hi all,

This patch series replaces manual cleanup and explicit kfree() calls with the __free attribute from <linux/cleanup.h>. This modernizes the memory management style and simplifies common error paths without altering any functional behavior.

The __free attribute provides automatic scope-based cleanup, making resource management clearer and reducing the chances of missing cleanup on early returns.

No functional changes are intended in this series.

Quoted 8 lines

Testing:
- Compiled with W=1
- Build-tested on i86_64

Based on:
<linux-v7.0-rc2>

Feel free to share your valuable input in context of the cleanup API.

Do you put random people in the Cc list?
You may try my script [1] to see the difference.

[1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh

Signature · 2 lines
With Best Regards,
Andy Shevchenko
reviewer
#19 lore raw
In reply to Sanjay Chitroda
Quoted 27 lines · Sanjay Chitroda

From: Sanjay Chitroda <[email protected]>

Hi all,

This patch series replaces manual cleanup and explicit kfree() calls with the __free attribute from <linux/cleanup.h>. This modernizes the memory management style and simplifies common error paths without altering any functional behavior.

The __free attribute provides automatic scope-based cleanup, making resource management clearer and reducing the chances of missing cleanup on early returns.

No functional changes are intended in this series.

Testing:
- Compiled with W=1
- Build-tested on i86_64

Based on:
<linux-v7.0-rc2>

Feel free to share your valuable input in context of the cleanup API.

Thanks,
Sanjay Chitroda

Quoted 8 lines

Sanjay Chitroda (7):
staging: greybus: simplify cleanup using __free
iio: ssp_sensors: simplify cleanup using __free
iio: st_sensors: simplify cleanup using __free
media: mediatek: vcodec: simplify cleanup using __free
media: chips-media: coda: simplify cleanup using __free
media: allegro: simplify cleanup using __free
staging: rtl8723bs: simplify cleanup using __free

There is no reason to put patches from different subsystems in the same series when there is no dependency between them. It just make for more noise for everyone.

author
#20 lore raw
Quoted 37 lines · David Lechner
Quoted 27 lines · Sanjay Chitroda

From: Sanjay Chitroda <[email protected]>

Hi all,

This patch series replaces manual cleanup and explicit kfree() calls with the __free attribute from <linux/cleanup.h>. This modernizes the memory management style and simplifies common error paths without altering any functional behavior.

The __free attribute provides automatic scope-based cleanup, making resource management clearer and reducing the chances of missing cleanup on early returns.

No functional changes are intended in this series.

Testing:
- Compiled with W=1
- Build-tested on i86_64

Based on:
<linux-v7.0-rc2>

Feel free to share your valuable input in context of the cleanup API.

Thanks,
Sanjay Chitroda

Quoted 8 lines

Sanjay Chitroda (7):
staging: greybus: simplify cleanup using __free
iio: ssp_sensors: simplify cleanup using __free
iio: st_sensors: simplify cleanup using __free
media: mediatek: vcodec: simplify cleanup using __free
media: chips-media: coda: simplify cleanup using __free
media: allegro: simplify cleanup using __free
staging: rtl8723bs: simplify cleanup using __free

Quoted 4 lines

There is no reason to put patches from different subsystems in the same series when there is no dependency between them. It just make for more noise for everyone.

Thanks for the feedback. I will split unrelated patches into separate series per subsystem or individual in future submissions.

#21 lore raw
In reply to Sanjay Chitroda

Dear Sanjay,
Would you mind explaining why you put so many people into TO and CC fields when sending patch series. You should stick to maintainers and reviewers from maintainers file as others can still see it on the mailing list(that's why you CC kernel mailing.list related emails). Sincerely Luka Gejak
P.S. If you are including me in further discussions or patch series please use [email protected] instead of [email protected]. Thanks in advance.

Quoted 53 lines · Sanjay Chitroda

From: Sanjay Chitroda <[email protected]>

Hi all,

This patch series replaces manual cleanup and explicit kfree() calls with the __free attribute from <linux/cleanup.h>. This modernizes the memory management style and simplifies common error paths without altering any functional behavior.

The __free attribute provides automatic scope-based cleanup, making resource management clearer and reducing the chances of missing cleanup on early returns.

No functional changes are intended in this series.

Testing:
- Compiled with W=1
- Build-tested on i86_64

Based on:
<linux-v7.0-rc2>

Feel free to share your valuable input in context of the cleanup API.

Thanks,
Sanjay Chitroda

Sanjay Chitroda (7):
staging: greybus: simplify cleanup using __free
iio: ssp_sensors: simplify cleanup using __free
iio: st_sensors: simplify cleanup using __free
media: mediatek: vcodec: simplify cleanup using __free
media: chips-media: coda: simplify cleanup using __free
media: allegro: simplify cleanup using __free
staging: rtl8723bs: simplify cleanup using __free

 drivers/iio/common/ssp_sensors/ssp_spi.c      |  9 +-
 .../iio/common/st_sensors/st_sensors_core.c   |  7 +-
 .../media/platform/allegro-dvt/allegro-core.c | 95 +++++--------------
 .../platform/chips-media/coda/coda-bit.c      |  4 +-
 .../platform/chips-media/coda/coda-jpeg.c     | 39 ++++----
 .../mediatek/vcodec/common/mtk_vcodec_dbgfs.c |  3 +-
 drivers/staging/greybus/camera.c              | 27 ++----
 drivers/staging/greybus/loopback.c            | 35 +++-----
 drivers/staging/greybus/raw.c                 |  6 +-
 .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 13 +--
 drivers/staging/rtl8723bs/hal/sdio_ops.c      | 37 ++------
 11 files changed, 78 insertions(+), 197 deletions(-)

--
2.34.1