Linux Sound subsystem development
 help / color / mirror / Atom feed
* [PATCH 0/9] ALSA: Use automatic cleanup of kfree()
@ 2024-02-22 11:15 Takashi Iwai
  2024-02-22 11:15 ` [PATCH 1/9] ALSA: pcm: " Takashi Iwai
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Takashi Iwai @ 2024-02-22 11:15 UTC (permalink / raw)
  To: linux-sound

Hi,

this is a series of patches for ALSA core code to use the new cleanup
mechanism for temporary buffers.
Merely a cleanup using cleanup, no functional changes.
I confirmed that there is no significant resultant size grow by this,
too.


Takashi

===

Takashi Iwai (9):
  ALSA: pcm: Use automatic cleanup of kfree()
  ALSA: control: Use automatic cleanup of kfree()
  ALSA: compress_offload: Use automatic cleanup of kfree()
  ALSA: timer: Use automatic cleanup of kfree()
  ALSA: vmaster: Use automatic cleanup of kfree()
  ALSA: seq: oss: Use automatic cleanup of kfree()
  ALSA: seq: virmidi: Use automatic cleanup of kfree()
  ALSA: seq: ump: Use automatic cleanup of kfree()
  ALSA: seq: core: Use automatic cleanup of kfree()

 sound/core/compress_offload.c     | 36 ++++-------
 sound/core/control.c              | 23 +++----
 sound/core/control_compat.c       | 69 ++++++++-------------
 sound/core/pcm.c                  |  4 +-
 sound/core/pcm_compat.c           | 29 ++++-----
 sound/core/pcm_native.c           | 99 ++++++++++++-------------------
 sound/core/seq/oss/seq_oss_init.c | 15 ++---
 sound/core/seq/oss/seq_oss_midi.c | 11 +---
 sound/core/seq/seq_compat.c       | 12 ++--
 sound/core/seq/seq_midi.c         | 14 +----
 sound/core/seq/seq_ump_client.c   | 33 ++++-------
 sound/core/seq/seq_virmidi.c      | 22 +++----
 sound/core/timer.c                | 13 ++--
 sound/core/vmaster.c              | 19 ++----
 14 files changed, 142 insertions(+), 257 deletions(-)

-- 
2.35.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/9] ALSA: pcm: Use automatic cleanup of kfree()
  2024-02-22 11:15 [PATCH 0/9] ALSA: Use automatic cleanup of kfree() Takashi Iwai
@ 2024-02-22 11:15 ` Takashi Iwai
  2024-02-22 11:15 ` [PATCH 2/9] ALSA: control: " Takashi Iwai
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2024-02-22 11:15 UTC (permalink / raw)
  To: linux-sound

There are common patterns where a temporary buffer is allocated and
freed at the exit, and those can be simplified with the recent cleanup
mechanism via __free(kfree).

A caveat is that some allocations are memdup_user() and they return an
error pointer instead of NULL.  Those need special cares and the value
has to be cleared with no_free_ptr() at the allocation error path.

Other than that, the conversions are straightforward.

No functional changes, only code refactoring.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/pcm.c        |  4 +-
 sound/core/pcm_compat.c | 29 +++++-------
 sound/core/pcm_native.c | 99 ++++++++++++++++-------------------------
 3 files changed, 49 insertions(+), 83 deletions(-)

diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index d9b338088d10..87d27fbdfe5c 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -342,7 +342,7 @@ static const char *snd_pcm_oss_format_name(int format)
 static void snd_pcm_proc_info_read(struct snd_pcm_substream *substream,
 				   struct snd_info_buffer *buffer)
 {
-	struct snd_pcm_info *info;
+	struct snd_pcm_info *info __free(kfree) = NULL;
 	int err;
 
 	if (! substream)
@@ -355,7 +355,6 @@ static void snd_pcm_proc_info_read(struct snd_pcm_substream *substream,
 	err = snd_pcm_info(substream, info);
 	if (err < 0) {
 		snd_iprintf(buffer, "error %d\n", err);
-		kfree(info);
 		return;
 	}
 	snd_iprintf(buffer, "card: %d\n", info->card);
@@ -369,7 +368,6 @@ static void snd_pcm_proc_info_read(struct snd_pcm_substream *substream,
 	snd_iprintf(buffer, "subclass: %d\n", info->dev_subclass);
 	snd_iprintf(buffer, "subdevices_count: %d\n", info->subdevices_count);
 	snd_iprintf(buffer, "subdevices_avail: %d\n", info->subdevices_avail);
-	kfree(info);
 }
 
 static void snd_pcm_stream_proc_info_read(struct snd_info_entry *entry,
diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c
index c96483091f30..ef3c0d177510 100644
--- a/sound/core/pcm_compat.c
+++ b/sound/core/pcm_compat.c
@@ -235,7 +235,7 @@ static int snd_pcm_ioctl_hw_params_compat(struct snd_pcm_substream *substream,
 					  int refine, 
 					  struct snd_pcm_hw_params32 __user *data32)
 {
-	struct snd_pcm_hw_params *data;
+	struct snd_pcm_hw_params *data __free(kfree) = NULL;
 	struct snd_pcm_runtime *runtime;
 	int err;
 
@@ -248,34 +248,28 @@ static int snd_pcm_ioctl_hw_params_compat(struct snd_pcm_substream *substream,
 		return -ENOMEM;
 
 	/* only fifo_size (RO from userspace) is different, so just copy all */
-	if (copy_from_user(data, data32, sizeof(*data32))) {
-		err = -EFAULT;
-		goto error;
-	}
+	if (copy_from_user(data, data32, sizeof(*data32)))
+		return -EFAULT;
 
 	if (refine) {
 		err = snd_pcm_hw_refine(substream, data);
 		if (err < 0)
-			goto error;
+			return err;
 		err = fixup_unreferenced_params(substream, data);
 	} else {
 		err = snd_pcm_hw_params(substream, data);
 	}
 	if (err < 0)
-		goto error;
+		return err;
 	if (copy_to_user(data32, data, sizeof(*data32)) ||
-	    put_user(data->fifo_size, &data32->fifo_size)) {
-		err = -EFAULT;
-		goto error;
-	}
+	    put_user(data->fifo_size, &data32->fifo_size))
+		return -EFAULT;
 
 	if (! refine) {
 		unsigned int new_boundary = recalculate_boundary(runtime);
 		if (new_boundary)
 			runtime->boundary = new_boundary;
 	}
- error:
-	kfree(data);
 	return err;
 }
 
@@ -338,7 +332,7 @@ static int snd_pcm_ioctl_xfern_compat(struct snd_pcm_substream *substream,
 	compat_caddr_t buf;
 	compat_caddr_t __user *bufptr;
 	u32 frames;
-	void __user **bufs;
+	void __user **bufs __free(kfree) = NULL;
 	int err, ch, i;
 
 	if (! substream->runtime)
@@ -360,10 +354,8 @@ static int snd_pcm_ioctl_xfern_compat(struct snd_pcm_substream *substream,
 		return -ENOMEM;
 	for (i = 0; i < ch; i++) {
 		u32 ptr;
-		if (get_user(ptr, bufptr)) {
-			kfree(bufs);
+		if (get_user(ptr, bufptr))
 			return -EFAULT;
-		}
 		bufs[i] = compat_ptr(ptr);
 		bufptr++;
 	}
@@ -373,9 +365,8 @@ static int snd_pcm_ioctl_xfern_compat(struct snd_pcm_substream *substream,
 		err = snd_pcm_lib_readv(substream, bufs, frames);
 	if (err >= 0) {
 		if (put_user(err, &data32->result))
-			err = -EFAULT;
+			return -EFAULT;
 	}
-	kfree(bufs);
 	return err;
 }
 
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index f5ff00f99788..beee5249dae1 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -236,7 +236,7 @@ int snd_pcm_info(struct snd_pcm_substream *substream, struct snd_pcm_info *info)
 int snd_pcm_info_user(struct snd_pcm_substream *substream,
 		      struct snd_pcm_info __user * _info)
 {
-	struct snd_pcm_info *info;
+	struct snd_pcm_info *info __free(kfree) = NULL;
 	int err;
 
 	info = kmalloc(sizeof(*info), GFP_KERNEL);
@@ -247,7 +247,6 @@ int snd_pcm_info_user(struct snd_pcm_substream *substream,
 		if (copy_to_user(_info, info, sizeof(*info)))
 			err = -EFAULT;
 	}
-	kfree(info);
 	return err;
 }
 
@@ -359,7 +358,7 @@ static int constrain_params_by_rules(struct snd_pcm_substream *substream,
 	struct snd_pcm_hw_constraints *constrs =
 					&substream->runtime->hw_constraints;
 	unsigned int k;
-	unsigned int *rstamps;
+	unsigned int *rstamps __free(kfree) = NULL;
 	unsigned int vstamps[SNDRV_PCM_HW_PARAM_LAST_INTERVAL + 1];
 	unsigned int stamp;
 	struct snd_pcm_hw_rule *r;
@@ -435,10 +434,8 @@ static int constrain_params_by_rules(struct snd_pcm_substream *substream,
 		}
 
 		changed = r->func(params, r);
-		if (changed < 0) {
-			err = changed;
-			goto out;
-		}
+		if (changed < 0)
+			return changed;
 
 		/*
 		 * When the parameter is changed, notify it to the caller
@@ -469,8 +466,6 @@ static int constrain_params_by_rules(struct snd_pcm_substream *substream,
 	if (again)
 		goto retry;
 
- out:
-	kfree(rstamps);
 	return err;
 }
 
@@ -571,26 +566,24 @@ EXPORT_SYMBOL(snd_pcm_hw_refine);
 static int snd_pcm_hw_refine_user(struct snd_pcm_substream *substream,
 				  struct snd_pcm_hw_params __user * _params)
 {
-	struct snd_pcm_hw_params *params;
+	struct snd_pcm_hw_params *params __free(kfree) = NULL;
 	int err;
 
 	params = memdup_user(_params, sizeof(*params));
 	if (IS_ERR(params))
-		return PTR_ERR(params);
+		return PTR_ERR(no_free_ptr(params));
 
 	err = snd_pcm_hw_refine(substream, params);
 	if (err < 0)
-		goto end;
+		return err;
 
 	err = fixup_unreferenced_params(substream, params);
 	if (err < 0)
-		goto end;
+		return err;
 
 	if (copy_to_user(_params, params, sizeof(*params)))
-		err = -EFAULT;
-end:
-	kfree(params);
-	return err;
+		return -EFAULT;
+	return 0;
 }
 
 static int period_to_usecs(struct snd_pcm_runtime *runtime)
@@ -864,21 +857,19 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
 static int snd_pcm_hw_params_user(struct snd_pcm_substream *substream,
 				  struct snd_pcm_hw_params __user * _params)
 {
-	struct snd_pcm_hw_params *params;
+	struct snd_pcm_hw_params *params __free(kfree) = NULL;
 	int err;
 
 	params = memdup_user(_params, sizeof(*params));
 	if (IS_ERR(params))
-		return PTR_ERR(params);
+		return PTR_ERR(no_free_ptr(params));
 
 	err = snd_pcm_hw_params(substream, params);
 	if (err < 0)
-		goto end;
+		return err;
 
 	if (copy_to_user(_params, params, sizeof(*params)))
-		err = -EFAULT;
-end:
-	kfree(params);
+		return -EFAULT;
 	return err;
 }
 
@@ -2271,7 +2262,8 @@ static int snd_pcm_link(struct snd_pcm_substream *substream, int fd)
 	int res = 0;
 	struct snd_pcm_file *pcm_file;
 	struct snd_pcm_substream *substream1;
-	struct snd_pcm_group *group, *target_group;
+	struct snd_pcm_group *group __free(kfree) = NULL;
+	struct snd_pcm_group *target_group;
 	bool nonatomic = substream->pcm->nonatomic;
 	struct fd f = fdget(fd);
 
@@ -2281,6 +2273,7 @@ static int snd_pcm_link(struct snd_pcm_substream *substream, int fd)
 		res = -EBADFD;
 		goto _badf;
 	}
+
 	pcm_file = f.file->private_data;
 	substream1 = pcm_file->substream;
 
@@ -2292,8 +2285,9 @@ static int snd_pcm_link(struct snd_pcm_substream *substream, int fd)
 	group = kzalloc(sizeof(*group), GFP_KERNEL);
 	if (!group) {
 		res = -ENOMEM;
-		goto _nolock;
+		goto _badf;
 	}
+
 	snd_pcm_group_init(group);
 
 	down_write(&snd_pcm_link_rwsem);
@@ -2324,8 +2318,6 @@ static int snd_pcm_link(struct snd_pcm_substream *substream, int fd)
 	snd_pcm_group_unlock_irq(target_group, nonatomic);
  _end:
 	up_write(&snd_pcm_link_rwsem);
- _nolock:
-	kfree(group);
  _badf:
 	fdput(f);
 	return res;
@@ -3279,7 +3271,7 @@ static int snd_pcm_xfern_frames_ioctl(struct snd_pcm_substream *substream,
 {
 	struct snd_xfern xfern;
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	void *bufs;
+	void *bufs __free(kfree) = NULL;
 	snd_pcm_sframes_t result;
 
 	if (runtime->state == SNDRV_PCM_STATE_OPEN)
@@ -3293,12 +3285,11 @@ static int snd_pcm_xfern_frames_ioctl(struct snd_pcm_substream *substream,
 
 	bufs = memdup_user(xfern.bufs, sizeof(void *) * runtime->channels);
 	if (IS_ERR(bufs))
-		return PTR_ERR(bufs);
+		return PTR_ERR(no_free_ptr(bufs));
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		result = snd_pcm_lib_writev(substream, bufs, xfern.frames);
 	else
 		result = snd_pcm_lib_readv(substream, bufs, xfern.frames);
-	kfree(bufs);
 	if (put_user(result, &_xfern->result))
 		return -EFAULT;
 	return result < 0 ? result : 0;
@@ -3566,7 +3557,7 @@ static ssize_t snd_pcm_readv(struct kiocb *iocb, struct iov_iter *to)
 	struct snd_pcm_runtime *runtime;
 	snd_pcm_sframes_t result;
 	unsigned long i;
-	void __user **bufs;
+	void __user **bufs __free(kfree) = NULL;
 	snd_pcm_uframes_t frames;
 	const struct iovec *iov = iter_iov(to);
 
@@ -3595,7 +3586,6 @@ static ssize_t snd_pcm_readv(struct kiocb *iocb, struct iov_iter *to)
 	result = snd_pcm_lib_readv(substream, bufs, frames);
 	if (result > 0)
 		result = frames_to_bytes(runtime, result);
-	kfree(bufs);
 	return result;
 }
 
@@ -3606,7 +3596,7 @@ static ssize_t snd_pcm_writev(struct kiocb *iocb, struct iov_iter *from)
 	struct snd_pcm_runtime *runtime;
 	snd_pcm_sframes_t result;
 	unsigned long i;
-	void __user **bufs;
+	void __user **bufs __free(kfree) = NULL;
 	snd_pcm_uframes_t frames;
 	const struct iovec *iov = iter_iov(from);
 
@@ -3634,7 +3624,6 @@ static ssize_t snd_pcm_writev(struct kiocb *iocb, struct iov_iter *from)
 	result = snd_pcm_lib_writev(substream, bufs, frames);
 	if (result > 0)
 		result = frames_to_bytes(runtime, result);
-	kfree(bufs);
 	return result;
 }
 
@@ -4076,8 +4065,8 @@ static void snd_pcm_hw_convert_to_old_params(struct snd_pcm_hw_params_old *opara
 static int snd_pcm_hw_refine_old_user(struct snd_pcm_substream *substream,
 				      struct snd_pcm_hw_params_old __user * _oparams)
 {
-	struct snd_pcm_hw_params *params;
-	struct snd_pcm_hw_params_old *oparams = NULL;
+	struct snd_pcm_hw_params *params __free(kfree) = NULL;
+	struct snd_pcm_hw_params_old *oparams __free(kfree) = NULL;
 	int err;
 
 	params = kmalloc(sizeof(*params), GFP_KERNEL);
@@ -4085,34 +4074,28 @@ static int snd_pcm_hw_refine_old_user(struct snd_pcm_substream *substream,
 		return -ENOMEM;
 
 	oparams = memdup_user(_oparams, sizeof(*oparams));
-	if (IS_ERR(oparams)) {
-		err = PTR_ERR(oparams);
-		goto out;
-	}
+	if (IS_ERR(oparams))
+		return PTR_ERR(no_free_ptr(oparams));
 	snd_pcm_hw_convert_from_old_params(params, oparams);
 	err = snd_pcm_hw_refine(substream, params);
 	if (err < 0)
-		goto out_old;
+		return err;
 
 	err = fixup_unreferenced_params(substream, params);
 	if (err < 0)
-		goto out_old;
+		return err;
 
 	snd_pcm_hw_convert_to_old_params(oparams, params);
 	if (copy_to_user(_oparams, oparams, sizeof(*oparams)))
-		err = -EFAULT;
-out_old:
-	kfree(oparams);
-out:
-	kfree(params);
-	return err;
+		return -EFAULT;
+	return 0;
 }
 
 static int snd_pcm_hw_params_old_user(struct snd_pcm_substream *substream,
 				      struct snd_pcm_hw_params_old __user * _oparams)
 {
-	struct snd_pcm_hw_params *params;
-	struct snd_pcm_hw_params_old *oparams = NULL;
+	struct snd_pcm_hw_params *params __free(kfree) = NULL;
+	struct snd_pcm_hw_params_old *oparams __free(kfree) = NULL;
 	int err;
 
 	params = kmalloc(sizeof(*params), GFP_KERNEL);
@@ -4120,24 +4103,18 @@ static int snd_pcm_hw_params_old_user(struct snd_pcm_substream *substream,
 		return -ENOMEM;
 
 	oparams = memdup_user(_oparams, sizeof(*oparams));
-	if (IS_ERR(oparams)) {
-		err = PTR_ERR(oparams);
-		goto out;
-	}
+	if (IS_ERR(oparams))
+		return PTR_ERR(no_free_ptr(oparams));
 
 	snd_pcm_hw_convert_from_old_params(params, oparams);
 	err = snd_pcm_hw_params(substream, params);
 	if (err < 0)
-		goto out_old;
+		return err;
 
 	snd_pcm_hw_convert_to_old_params(oparams, params);
 	if (copy_to_user(_oparams, oparams, sizeof(*oparams)))
-		err = -EFAULT;
-out_old:
-	kfree(oparams);
-out:
-	kfree(params);
-	return err;
+		return -EFAULT;
+	return 0;
 }
 #endif /* CONFIG_SND_SUPPORT_OLD_API */
 
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/9] ALSA: control: Use automatic cleanup of kfree()
  2024-02-22 11:15 [PATCH 0/9] ALSA: Use automatic cleanup of kfree() Takashi Iwai
  2024-02-22 11:15 ` [PATCH 1/9] ALSA: pcm: " Takashi Iwai
@ 2024-02-22 11:15 ` Takashi Iwai
  2024-02-22 11:15 ` [PATCH 3/9] ALSA: compress_offload: " Takashi Iwai
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2024-02-22 11:15 UTC (permalink / raw)
  To: linux-sound

There are common patterns where a temporary buffer is allocated and
freed at the exit, and those can be simplified with the recent cleanup
mechanism via __free(kfree).

A caveat is that some allocations are memdup_user() and they return an
error pointer instead of NULL.  Those need special cares and the value
has to be cleared with no_free_ptr() at the allocation error path.

Other than that, the conversions are straightforward.

No functional changes, only code refactoring.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/control.c        | 23 +++++--------
 sound/core/control_compat.c | 69 ++++++++++++++-----------------------
 2 files changed, 34 insertions(+), 58 deletions(-)

diff --git a/sound/core/control.c b/sound/core/control.c
index 59c8658966d4..c8cd70aed6af 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -932,7 +932,7 @@ EXPORT_SYMBOL(snd_ctl_find_id);
 static int snd_ctl_card_info(struct snd_card *card, struct snd_ctl_file * ctl,
 			     unsigned int cmd, void __user *arg)
 {
-	struct snd_ctl_card_info *info;
+	struct snd_ctl_card_info *info __free(kfree) = NULL;
 
 	info = kzalloc(sizeof(*info), GFP_KERNEL);
 	if (! info)
@@ -946,11 +946,8 @@ static int snd_ctl_card_info(struct snd_card *card, struct snd_ctl_file * ctl,
 	strscpy(info->mixername, card->mixername, sizeof(info->mixername));
 	strscpy(info->components, card->components, sizeof(info->components));
 	up_read(&snd_ioctl_rwsem);
-	if (copy_to_user(arg, info, sizeof(struct snd_ctl_card_info))) {
-		kfree(info);
+	if (copy_to_user(arg, info, sizeof(struct snd_ctl_card_info)))
 		return -EFAULT;
-	}
-	kfree(info);
 	return 0;
 }
 
@@ -1339,12 +1336,10 @@ static int snd_ctl_elem_read_user(struct snd_card *card,
 
 	result = snd_ctl_elem_read(card, control);
 	if (result < 0)
-		goto error;
+		return result;
 
 	if (copy_to_user(_control, control, sizeof(*control)))
-		result = -EFAULT;
- error:
-	kfree(control);
+		return -EFAULT;
 	return result;
 }
 
@@ -1406,23 +1401,21 @@ static int snd_ctl_elem_write(struct snd_card *card, struct snd_ctl_file *file,
 static int snd_ctl_elem_write_user(struct snd_ctl_file *file,
 				   struct snd_ctl_elem_value __user *_control)
 {
-	struct snd_ctl_elem_value *control;
+	struct snd_ctl_elem_value *control __free(kfree) = NULL;
 	struct snd_card *card;
 	int result;
 
 	control = memdup_user(_control, sizeof(*control));
 	if (IS_ERR(control))
-		return PTR_ERR(control);
+		return PTR_ERR(no_free_ptr(control));
 
 	card = file->card;
 	result = snd_ctl_elem_write(card, file, control);
 	if (result < 0)
-		goto error;
+		return result;
 
 	if (copy_to_user(_control, control, sizeof(*control)))
-		result = -EFAULT;
- error:
-	kfree(control);
+		return -EFAULT;
 	return result;
 }
 
diff --git a/sound/core/control_compat.c b/sound/core/control_compat.c
index 63d787501066..8392183c77ed 100644
--- a/sound/core/control_compat.c
+++ b/sound/core/control_compat.c
@@ -79,61 +79,56 @@ struct snd_ctl_elem_info32 {
 static int snd_ctl_elem_info_compat(struct snd_ctl_file *ctl,
 				    struct snd_ctl_elem_info32 __user *data32)
 {
-	struct snd_ctl_elem_info *data;
+	struct snd_ctl_elem_info *data __free(kfree) = NULL;
 	int err;
 
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (! data)
 		return -ENOMEM;
 
-	err = -EFAULT;
 	/* copy id */
 	if (copy_from_user(&data->id, &data32->id, sizeof(data->id)))
-		goto error;
+		return -EFAULT;
 	/* we need to copy the item index.
 	 * hope this doesn't break anything..
 	 */
 	if (get_user(data->value.enumerated.item, &data32->value.enumerated.item))
-		goto error;
+		return -EFAULT;
 
 	err = snd_ctl_elem_info(ctl, data);
 	if (err < 0)
-		goto error;
+		return err;
 	/* restore info to 32bit */
-	err = -EFAULT;
 	/* id, type, access, count */
 	if (copy_to_user(&data32->id, &data->id, sizeof(data->id)) ||
 	    copy_to_user(&data32->type, &data->type, 3 * sizeof(u32)))
-		goto error;
+		return -EFAULT;
 	if (put_user(data->owner, &data32->owner))
-		goto error;
+		return -EFAULT;
 	switch (data->type) {
 	case SNDRV_CTL_ELEM_TYPE_BOOLEAN:
 	case SNDRV_CTL_ELEM_TYPE_INTEGER:
 		if (put_user(data->value.integer.min, &data32->value.integer.min) ||
 		    put_user(data->value.integer.max, &data32->value.integer.max) ||
 		    put_user(data->value.integer.step, &data32->value.integer.step))
-			goto error;
+			return -EFAULT;
 		break;
 	case SNDRV_CTL_ELEM_TYPE_INTEGER64:
 		if (copy_to_user(&data32->value.integer64,
 				 &data->value.integer64,
 				 sizeof(data->value.integer64)))
-			goto error;
+			return -EFAULT;
 		break;
 	case SNDRV_CTL_ELEM_TYPE_ENUMERATED:
 		if (copy_to_user(&data32->value.enumerated,
 				 &data->value.enumerated,
 				 sizeof(data->value.enumerated)))
-			goto error;
+			return -EFAULT;
 		break;
 	default:
 		break;
 	}
-	err = 0;
- error:
-	kfree(data);
-	return err;
+	return 0;
 }
 
 /* read / write */
@@ -169,7 +164,7 @@ static int get_ctl_type(struct snd_card *card, struct snd_ctl_elem_id *id,
 			int *countp)
 {
 	struct snd_kcontrol *kctl;
-	struct snd_ctl_elem_info *info;
+	struct snd_ctl_elem_info *info __free(kfree) = NULL;
 	int err;
 
 	down_read(&card->controls_rwsem);
@@ -193,7 +188,6 @@ static int get_ctl_type(struct snd_card *card, struct snd_ctl_elem_id *id,
 		err = info->type;
 		*countp = info->count;
 	}
-	kfree(info);
 	return err;
 }
 
@@ -289,7 +283,7 @@ static int copy_ctl_value_to_user(void __user *userdata,
 static int ctl_elem_read_user(struct snd_card *card,
 			      void __user *userdata, void __user *valuep)
 {
-	struct snd_ctl_elem_value *data;
+	struct snd_ctl_elem_value *data __free(kfree) = NULL;
 	int err, type, count;
 
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
@@ -299,21 +293,18 @@ static int ctl_elem_read_user(struct snd_card *card,
 	err = copy_ctl_value_from_user(card, data, userdata, valuep,
 				       &type, &count);
 	if (err < 0)
-		goto error;
+		return err;
 
 	err = snd_ctl_elem_read(card, data);
 	if (err < 0)
-		goto error;
-	err = copy_ctl_value_to_user(userdata, valuep, data, type, count);
- error:
-	kfree(data);
-	return err;
+		return err;
+	return copy_ctl_value_to_user(userdata, valuep, data, type, count);
 }
 
 static int ctl_elem_write_user(struct snd_ctl_file *file,
 			       void __user *userdata, void __user *valuep)
 {
-	struct snd_ctl_elem_value *data;
+	struct snd_ctl_elem_value *data __free(kfree) = NULL;
 	struct snd_card *card = file->card;
 	int err, type, count;
 
@@ -324,15 +315,12 @@ static int ctl_elem_write_user(struct snd_ctl_file *file,
 	err = copy_ctl_value_from_user(card, data, userdata, valuep,
 				       &type, &count);
 	if (err < 0)
-		goto error;
+		return err;
 
 	err = snd_ctl_elem_write(card, file, data);
 	if (err < 0)
-		goto error;
-	err = copy_ctl_value_to_user(userdata, valuep, data, type, count);
- error:
-	kfree(data);
-	return err;
+		return err;
+	return copy_ctl_value_to_user(userdata, valuep, data, type, count);
 }
 
 static int snd_ctl_elem_read_user_compat(struct snd_card *card,
@@ -366,49 +354,44 @@ static int snd_ctl_elem_add_compat(struct snd_ctl_file *file,
 				   struct snd_ctl_elem_info32 __user *data32,
 				   int replace)
 {
-	struct snd_ctl_elem_info *data;
-	int err;
+	struct snd_ctl_elem_info *data __free(kfree) = NULL;
 
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (! data)
 		return -ENOMEM;
 
-	err = -EFAULT;
 	/* id, type, access, count */ \
 	if (copy_from_user(&data->id, &data32->id, sizeof(data->id)) ||
 	    copy_from_user(&data->type, &data32->type, 3 * sizeof(u32)))
-		goto error;
+		return -EFAULT;
 	if (get_user(data->owner, &data32->owner))
-		goto error;
+		return -EFAULT;
 	switch (data->type) {
 	case SNDRV_CTL_ELEM_TYPE_BOOLEAN:
 	case SNDRV_CTL_ELEM_TYPE_INTEGER:
 		if (get_user(data->value.integer.min, &data32->value.integer.min) ||
 		    get_user(data->value.integer.max, &data32->value.integer.max) ||
 		    get_user(data->value.integer.step, &data32->value.integer.step))
-			goto error;
+			return -EFAULT;
 		break;
 	case SNDRV_CTL_ELEM_TYPE_INTEGER64:
 		if (copy_from_user(&data->value.integer64,
 				   &data32->value.integer64,
 				   sizeof(data->value.integer64)))
-			goto error;
+			return -EFAULT;
 		break;
 	case SNDRV_CTL_ELEM_TYPE_ENUMERATED:
 		if (copy_from_user(&data->value.enumerated,
 				   &data32->value.enumerated,
 				   sizeof(data->value.enumerated)))
-			goto error;
+			return -EFAULT;
 		data->value.enumerated.names_ptr =
 			(uintptr_t)compat_ptr(data->value.enumerated.names_ptr);
 		break;
 	default:
 		break;
 	}
-	err = snd_ctl_elem_add(file, data, replace);
- error:
-	kfree(data);
-	return err;
+	return snd_ctl_elem_add(file, data, replace);
 }  
 
 enum {
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/9] ALSA: compress_offload: Use automatic cleanup of kfree()
  2024-02-22 11:15 [PATCH 0/9] ALSA: Use automatic cleanup of kfree() Takashi Iwai
  2024-02-22 11:15 ` [PATCH 1/9] ALSA: pcm: " Takashi Iwai
  2024-02-22 11:15 ` [PATCH 2/9] ALSA: control: " Takashi Iwai
@ 2024-02-22 11:15 ` Takashi Iwai
  2024-02-22 11:15 ` [PATCH 4/9] ALSA: timer: " Takashi Iwai
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2024-02-22 11:15 UTC (permalink / raw)
  To: linux-sound

There are common patterns where a temporary buffer is allocated and
freed at the exit, and those can be simplified with the recent cleanup
mechanism via __free(kfree).

A caveat is that some allocations are memdup_user() and they return an
error pointer instead of NULL.  Those need special cares and the value
has to be cleared with no_free_ptr() at the allocation error path.

Other than that, the conversions are straightforward.

No functional changes, only code refactoring.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/compress_offload.c | 36 +++++++++++++----------------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index 619371aa9964..5d926c5b737d 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -465,7 +465,7 @@ static int
 snd_compr_get_codec_caps(struct snd_compr_stream *stream, unsigned long arg)
 {
 	int retval;
-	struct snd_compr_codec_caps *caps;
+	struct snd_compr_codec_caps *caps __free(kfree) = NULL;
 
 	if (!stream->ops->get_codec_caps)
 		return -ENXIO;
@@ -476,12 +476,9 @@ snd_compr_get_codec_caps(struct snd_compr_stream *stream, unsigned long arg)
 
 	retval = stream->ops->get_codec_caps(stream, caps);
 	if (retval)
-		goto out;
+		return retval;
 	if (copy_to_user((void __user *)arg, caps, sizeof(*caps)))
-		retval = -EFAULT;
-
-out:
-	kfree(caps);
+		return -EFAULT;
 	return retval;
 }
 #endif /* !COMPR_CODEC_CAPS_OVERFLOW */
@@ -586,7 +583,7 @@ static int snd_compress_check_input(struct snd_compr_params *params)
 static int
 snd_compr_set_params(struct snd_compr_stream *stream, unsigned long arg)
 {
-	struct snd_compr_params *params;
+	struct snd_compr_params *params __free(kfree) = NULL;
 	int retval;
 
 	if (stream->runtime->state == SNDRV_PCM_STATE_OPEN || stream->next_track) {
@@ -596,24 +593,22 @@ snd_compr_set_params(struct snd_compr_stream *stream, unsigned long arg)
 		 */
 		params = memdup_user((void __user *)arg, sizeof(*params));
 		if (IS_ERR(params))
-			return PTR_ERR(params);
+			return PTR_ERR(no_free_ptr(params));
 
 		retval = snd_compress_check_input(params);
 		if (retval)
-			goto out;
+			return retval;
 
 		retval = snd_compr_allocate_buffer(stream, params);
-		if (retval) {
-			retval = -ENOMEM;
-			goto out;
-		}
+		if (retval)
+			return -ENOMEM;
 
 		retval = stream->ops->set_params(stream, params);
 		if (retval)
-			goto out;
+			return retval;
 
 		if (stream->next_track)
-			goto out;
+			return retval;
 
 		stream->metadata_set = false;
 		stream->next_track = false;
@@ -622,15 +617,13 @@ snd_compr_set_params(struct snd_compr_stream *stream, unsigned long arg)
 	} else {
 		return -EPERM;
 	}
-out:
-	kfree(params);
 	return retval;
 }
 
 static int
 snd_compr_get_params(struct snd_compr_stream *stream, unsigned long arg)
 {
-	struct snd_codec *params;
+	struct snd_codec *params __free(kfree) = NULL;
 	int retval;
 
 	if (!stream->ops->get_params)
@@ -641,12 +634,9 @@ snd_compr_get_params(struct snd_compr_stream *stream, unsigned long arg)
 		return -ENOMEM;
 	retval = stream->ops->get_params(stream, params);
 	if (retval)
-		goto out;
+		return retval;
 	if (copy_to_user((char __user *)arg, params, sizeof(*params)))
-		retval = -EFAULT;
-
-out:
-	kfree(params);
+		return -EFAULT;
 	return retval;
 }
 
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/9] ALSA: timer: Use automatic cleanup of kfree()
  2024-02-22 11:15 [PATCH 0/9] ALSA: Use automatic cleanup of kfree() Takashi Iwai
                   ` (2 preceding siblings ...)
  2024-02-22 11:15 ` [PATCH 3/9] ALSA: compress_offload: " Takashi Iwai
@ 2024-02-22 11:15 ` Takashi Iwai
  2024-02-22 11:15 ` [PATCH 5/9] ALSA: vmaster: " Takashi Iwai
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2024-02-22 11:15 UTC (permalink / raw)
  To: linux-sound

There are common patterns where a temporary buffer is allocated and
freed at the exit, and those can be simplified with the recent cleanup
mechanism via __free(kfree).

No functional changes, only code refactoring.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/timer.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/sound/core/timer.c b/sound/core/timer.c
index e6e551d4a29e..a595c4fb4b2a 100644
--- a/sound/core/timer.c
+++ b/sound/core/timer.c
@@ -1645,7 +1645,7 @@ static int snd_timer_user_next_device(struct snd_timer_id __user *_tid)
 static int snd_timer_user_ginfo(struct file *file,
 				struct snd_timer_ginfo __user *_ginfo)
 {
-	struct snd_timer_ginfo *ginfo;
+	struct snd_timer_ginfo *ginfo __free(kfree) = NULL;
 	struct snd_timer_id tid;
 	struct snd_timer *t;
 	struct list_head *p;
@@ -1653,7 +1653,7 @@ static int snd_timer_user_ginfo(struct file *file,
 
 	ginfo = memdup_user(_ginfo, sizeof(*ginfo));
 	if (IS_ERR(ginfo))
-		return PTR_ERR(ginfo);
+		return PTR_ERR(no_free_ptr(ginfo));
 
 	tid = ginfo->tid;
 	memset(ginfo, 0, sizeof(*ginfo));
@@ -1682,7 +1682,6 @@ static int snd_timer_user_ginfo(struct file *file,
 	mutex_unlock(&register_mutex);
 	if (err >= 0 && copy_to_user(_ginfo, ginfo, sizeof(*ginfo)))
 		err = -EFAULT;
-	kfree(ginfo);
 	return err;
 }
 
@@ -1804,9 +1803,8 @@ static int snd_timer_user_info(struct file *file,
 			       struct snd_timer_info __user *_info)
 {
 	struct snd_timer_user *tu;
-	struct snd_timer_info *info;
+	struct snd_timer_info *info __free(kfree) = NULL;
 	struct snd_timer *t;
-	int err = 0;
 
 	tu = file->private_data;
 	if (!tu->timeri)
@@ -1827,9 +1825,8 @@ static int snd_timer_user_info(struct file *file,
 	info->resolution = snd_timer_hw_resolution(t);
 	spin_unlock_irq(&t->lock);
 	if (copy_to_user(_info, info, sizeof(*_info)))
-		err = -EFAULT;
-	kfree(info);
-	return err;
+		return -EFAULT;
+	return 0;
 }
 
 static int snd_timer_user_params(struct file *file,
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/9] ALSA: vmaster: Use automatic cleanup of kfree()
  2024-02-22 11:15 [PATCH 0/9] ALSA: Use automatic cleanup of kfree() Takashi Iwai
                   ` (3 preceding siblings ...)
  2024-02-22 11:15 ` [PATCH 4/9] ALSA: timer: " Takashi Iwai
@ 2024-02-22 11:15 ` Takashi Iwai
  2024-02-22 11:15 ` [PATCH 6/9] ALSA: seq: oss: " Takashi Iwai
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2024-02-22 11:15 UTC (permalink / raw)
  To: linux-sound

There are common patterns where a temporary buffer is allocated and
freed at the exit, and those can be simplified with the recent cleanup
mechanism via __free(kfree).

No functional changes, only code refactoring.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/vmaster.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/sound/core/vmaster.c b/sound/core/vmaster.c
index 378d2c7c3d4a..04a57f7be6ea 100644
--- a/sound/core/vmaster.c
+++ b/sound/core/vmaster.c
@@ -56,7 +56,7 @@ struct link_follower {
 
 static int follower_update(struct link_follower *follower)
 {
-	struct snd_ctl_elem_value *uctl;
+	struct snd_ctl_elem_value *uctl __free(kfree) = NULL;
 	int err, ch;
 
 	uctl = kzalloc(sizeof(*uctl), GFP_KERNEL);
@@ -65,18 +65,16 @@ static int follower_update(struct link_follower *follower)
 	uctl->id = follower->follower.id;
 	err = follower->follower.get(&follower->follower, uctl);
 	if (err < 0)
-		goto error;
+		return err;
 	for (ch = 0; ch < follower->info.count; ch++)
 		follower->vals[ch] = uctl->value.integer.value[ch];
- error:
-	kfree(uctl);
-	return err < 0 ? err : 0;
+	return 0;
 }
 
 /* get the follower ctl info and save the initial values */
 static int follower_init(struct link_follower *follower)
 {
-	struct snd_ctl_elem_info *uinfo;
+	struct snd_ctl_elem_info *uinfo __free(kfree) = NULL;
 	int err;
 
 	if (follower->info.count) {
@@ -91,22 +89,18 @@ static int follower_init(struct link_follower *follower)
 		return -ENOMEM;
 	uinfo->id = follower->follower.id;
 	err = follower->follower.info(&follower->follower, uinfo);
-	if (err < 0) {
-		kfree(uinfo);
+	if (err < 0)
 		return err;
-	}
 	follower->info.type = uinfo->type;
 	follower->info.count = uinfo->count;
 	if (follower->info.count > 2  ||
 	    (follower->info.type != SNDRV_CTL_ELEM_TYPE_INTEGER &&
 	     follower->info.type != SNDRV_CTL_ELEM_TYPE_BOOLEAN)) {
 		pr_err("ALSA: vmaster: invalid follower element\n");
-		kfree(uinfo);
 		return -EINVAL;
 	}
 	follower->info.min_val = uinfo->value.integer.min;
 	follower->info.max_val = uinfo->value.integer.max;
-	kfree(uinfo);
 
 	return follower_update(follower);
 }
@@ -341,7 +335,7 @@ static int master_get(struct snd_kcontrol *kcontrol,
 static int sync_followers(struct link_master *master, int old_val, int new_val)
 {
 	struct link_follower *follower;
-	struct snd_ctl_elem_value *uval;
+	struct snd_ctl_elem_value *uval __free(kfree) = NULL;
 
 	uval = kmalloc(sizeof(*uval), GFP_KERNEL);
 	if (!uval)
@@ -353,7 +347,6 @@ static int sync_followers(struct link_master *master, int old_val, int new_val)
 		master->val = new_val;
 		follower_put_val(follower, uval);
 	}
-	kfree(uval);
 	return 0;
 }
 
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/9] ALSA: seq: oss: Use automatic cleanup of kfree()
  2024-02-22 11:15 [PATCH 0/9] ALSA: Use automatic cleanup of kfree() Takashi Iwai
                   ` (4 preceding siblings ...)
  2024-02-22 11:15 ` [PATCH 5/9] ALSA: vmaster: " Takashi Iwai
@ 2024-02-22 11:15 ` Takashi Iwai
  2024-02-22 11:15 ` [PATCH 7/9] ALSA: seq: virmidi: " Takashi Iwai
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2024-02-22 11:15 UTC (permalink / raw)
  To: linux-sound

There are common patterns where a temporary buffer is allocated and
freed at the exit, and those can be simplified with the recent cleanup
mechanism via __free(kfree).

No functional changes, only code refactoring.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/oss/seq_oss_init.c | 15 +++++----------
 sound/core/seq/oss/seq_oss_midi.c | 11 +++--------
 2 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/sound/core/seq/oss/seq_oss_init.c b/sound/core/seq/oss/seq_oss_init.c
index 76bf41c26acd..676246fa02f1 100644
--- a/sound/core/seq/oss/seq_oss_init.c
+++ b/sound/core/seq/oss/seq_oss_init.c
@@ -63,20 +63,18 @@ int __init
 snd_seq_oss_create_client(void)
 {
 	int rc;
-	struct snd_seq_port_info *port;
+	struct snd_seq_port_info *port __free(kfree) = NULL;
 	struct snd_seq_port_callback port_callback;
 
 	port = kzalloc(sizeof(*port), GFP_KERNEL);
-	if (!port) {
-		rc = -ENOMEM;
-		goto __error;
-	}
+	if (!port)
+		return -ENOMEM;
 
 	/* create ALSA client */
 	rc = snd_seq_create_kernel_client(NULL, SNDRV_SEQ_CLIENT_OSS,
 					  "OSS sequencer");
 	if (rc < 0)
-		goto __error;
+		return rc;
 
 	system_client = rc;
 
@@ -104,14 +102,11 @@ snd_seq_oss_create_client(void)
 		subs.dest.port = system_port;
 		call_ctl(SNDRV_SEQ_IOCTL_SUBSCRIBE_PORT, &subs);
 	}
-	rc = 0;
 
 	/* look up midi devices */
 	schedule_work(&async_lookup_work);
 
- __error:
-	kfree(port);
-	return rc;
+	return 0;
 }
 
 
diff --git a/sound/core/seq/oss/seq_oss_midi.c b/sound/core/seq/oss/seq_oss_midi.c
index f2940b29595f..f8e247d9e5c9 100644
--- a/sound/core/seq/oss/seq_oss_midi.c
+++ b/sound/core/seq/oss/seq_oss_midi.c
@@ -64,16 +64,13 @@ static int send_midi_event(struct seq_oss_devinfo *dp, struct snd_seq_event *ev,
 int
 snd_seq_oss_midi_lookup_ports(int client)
 {
-	struct snd_seq_client_info *clinfo;
-	struct snd_seq_port_info *pinfo;
+	struct snd_seq_client_info *clinfo __free(kfree) = NULL;
+	struct snd_seq_port_info *pinfo __free(kfree) = NULL;
 
 	clinfo = kzalloc(sizeof(*clinfo), GFP_KERNEL);
 	pinfo = kzalloc(sizeof(*pinfo), GFP_KERNEL);
-	if (! clinfo || ! pinfo) {
-		kfree(clinfo);
-		kfree(pinfo);
+	if (!clinfo || !pinfo)
 		return -ENOMEM;
-	}
 	clinfo->client = -1;
 	while (snd_seq_kernel_client_ctl(client, SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT, clinfo) == 0) {
 		if (clinfo->client == client)
@@ -83,8 +80,6 @@ snd_seq_oss_midi_lookup_ports(int client)
 		while (snd_seq_kernel_client_ctl(client, SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, pinfo) == 0)
 			snd_seq_oss_midi_check_new_port(pinfo);
 	}
-	kfree(clinfo);
-	kfree(pinfo);
 	return 0;
 }
 
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 7/9] ALSA: seq: virmidi: Use automatic cleanup of kfree()
  2024-02-22 11:15 [PATCH 0/9] ALSA: Use automatic cleanup of kfree() Takashi Iwai
                   ` (5 preceding siblings ...)
  2024-02-22 11:15 ` [PATCH 6/9] ALSA: seq: oss: " Takashi Iwai
@ 2024-02-22 11:15 ` Takashi Iwai
  2024-02-22 11:15 ` [PATCH 8/9] ALSA: seq: ump: " Takashi Iwai
  2024-02-22 11:15 ` [PATCH 9/9] ALSA: seq: core: " Takashi Iwai
  8 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2024-02-22 11:15 UTC (permalink / raw)
  To: linux-sound

There are common patterns where a temporary buffer is allocated and
freed at the exit, and those can be simplified with the recent cleanup
mechanism via __free(kfree).

No functional changes, only code refactoring.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/seq_virmidi.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c
index 1678737f11be..35f93b43dd2a 100644
--- a/sound/core/seq/seq_virmidi.c
+++ b/sound/core/seq/seq_virmidi.c
@@ -363,26 +363,22 @@ static int snd_virmidi_dev_attach_seq(struct snd_virmidi_dev *rdev)
 {
 	int client;
 	struct snd_seq_port_callback pcallbacks;
-	struct snd_seq_port_info *pinfo;
+	struct snd_seq_port_info *pinfo __free(kfree) = NULL;
 	int err;
 
 	if (rdev->client >= 0)
 		return 0;
 
 	pinfo = kzalloc(sizeof(*pinfo), GFP_KERNEL);
-	if (!pinfo) {
-		err = -ENOMEM;
-		goto __error;
-	}
+	if (!pinfo)
+		return -ENOMEM;
 
 	client = snd_seq_create_kernel_client(rdev->card, rdev->device,
 					      "%s %d-%d", rdev->rmidi->name,
 					      rdev->card->number,
 					      rdev->device);
-	if (client < 0) {
-		err = client;
-		goto __error;
-	}
+	if (client < 0)
+		return client;
 	rdev->client = client;
 
 	/* create a port */
@@ -410,15 +406,11 @@ static int snd_virmidi_dev_attach_seq(struct snd_virmidi_dev *rdev)
 	if (err < 0) {
 		snd_seq_delete_kernel_client(client);
 		rdev->client = -1;
-		goto __error;
+		return err;
 	}
 
 	rdev->port = pinfo->addr.port;
-	err = 0; /* success */
-
- __error:
-	kfree(pinfo);
-	return err;
+	return 0; /* success */
 }
 
 
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 8/9] ALSA: seq: ump: Use automatic cleanup of kfree()
  2024-02-22 11:15 [PATCH 0/9] ALSA: Use automatic cleanup of kfree() Takashi Iwai
                   ` (6 preceding siblings ...)
  2024-02-22 11:15 ` [PATCH 7/9] ALSA: seq: virmidi: " Takashi Iwai
@ 2024-02-22 11:15 ` Takashi Iwai
  2024-02-22 11:15 ` [PATCH 9/9] ALSA: seq: core: " Takashi Iwai
  8 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2024-02-22 11:15 UTC (permalink / raw)
  To: linux-sound

There are common patterns where a temporary buffer is allocated and
freed at the exit, and those can be simplified with the recent cleanup
mechanism via __free(kfree).

No functional changes, only code refactoring.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/seq_ump_client.c | 33 ++++++++++++---------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/sound/core/seq/seq_ump_client.c b/sound/core/seq/seq_ump_client.c
index 2db371d79930..ccac2d3a9fbc 100644
--- a/sound/core/seq/seq_ump_client.c
+++ b/sound/core/seq/seq_ump_client.c
@@ -217,15 +217,12 @@ static void fill_port_info(struct snd_seq_port_info *port,
 static int seq_ump_group_init(struct seq_ump_client *client, int group_index)
 {
 	struct seq_ump_group *group = &client->groups[group_index];
-	struct snd_seq_port_info *port;
+	struct snd_seq_port_info *port __free(kfree) = NULL;
 	struct snd_seq_port_callback pcallbacks;
-	int err;
 
 	port = kzalloc(sizeof(*port), GFP_KERNEL);
-	if (!port) {
-		err = -ENOMEM;
-		goto error;
-	}
+	if (!port)
+		return -ENOMEM;
 
 	fill_port_info(port, client, group);
 	port->flags = SNDRV_SEQ_PORT_FLG_GIVEN_PORT;
@@ -238,24 +235,22 @@ static int seq_ump_group_init(struct seq_ump_client *client, int group_index)
 	pcallbacks.unuse = seq_ump_unuse;
 	pcallbacks.event_input = seq_ump_process_event;
 	port->kernel = &pcallbacks;
-	err = snd_seq_kernel_client_ctl(client->seq_client,
-					SNDRV_SEQ_IOCTL_CREATE_PORT,
-					port);
- error:
-	kfree(port);
-	return err;
+	return snd_seq_kernel_client_ctl(client->seq_client,
+					 SNDRV_SEQ_IOCTL_CREATE_PORT,
+					 port);
 }
 
 /* update the sequencer ports; called from notify_fb_change callback */
 static void update_port_infos(struct seq_ump_client *client)
 {
-	struct snd_seq_port_info *old, *new;
+	struct snd_seq_port_info *old __free(kfree) = NULL;
+	struct snd_seq_port_info *new __free(kfree) = NULL;
 	int i, err;
 
 	old = kzalloc(sizeof(*old), GFP_KERNEL);
 	new = kzalloc(sizeof(*new), GFP_KERNEL);
 	if (!old || !new)
-		goto error;
+		return;
 
 	for (i = 0; i < SNDRV_UMP_MAX_GROUPS; i++) {
 		old->addr.client = client->seq_client;
@@ -264,7 +259,7 @@ static void update_port_infos(struct seq_ump_client *client)
 						SNDRV_SEQ_IOCTL_GET_PORT_INFO,
 						old);
 		if (err < 0)
-			goto error;
+			return;
 		fill_port_info(new, client, &client->groups[i]);
 		if (old->capability == new->capability &&
 		    !strcmp(old->name, new->name))
@@ -273,13 +268,10 @@ static void update_port_infos(struct seq_ump_client *client)
 						SNDRV_SEQ_IOCTL_SET_PORT_INFO,
 						new);
 		if (err < 0)
-			goto error;
+			return;
 		/* notify to system port */
 		snd_seq_system_client_ev_port_change(client->seq_client, i);
 	}
- error:
-	kfree(new);
-	kfree(old);
 }
 
 /* update dir_bits and active flag for all groups in the client */
@@ -334,7 +326,7 @@ static void update_group_attrs(struct seq_ump_client *client)
 /* create a UMP Endpoint port */
 static int create_ump_endpoint_port(struct seq_ump_client *client)
 {
-	struct snd_seq_port_info *port;
+	struct snd_seq_port_info *port __free(kfree) = NULL;
 	struct snd_seq_port_callback pcallbacks;
 	unsigned int rawmidi_info = client->ump->core.info_flags;
 	int err;
@@ -383,7 +375,6 @@ static int create_ump_endpoint_port(struct seq_ump_client *client)
 	err = snd_seq_kernel_client_ctl(client->seq_client,
 					SNDRV_SEQ_IOCTL_CREATE_PORT,
 					port);
-	kfree(port);
 	return err;
 }
 
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 9/9] ALSA: seq: core: Use automatic cleanup of kfree()
  2024-02-22 11:15 [PATCH 0/9] ALSA: Use automatic cleanup of kfree() Takashi Iwai
                   ` (7 preceding siblings ...)
  2024-02-22 11:15 ` [PATCH 8/9] ALSA: seq: ump: " Takashi Iwai
@ 2024-02-22 11:15 ` Takashi Iwai
  8 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2024-02-22 11:15 UTC (permalink / raw)
  To: linux-sound

There are common patterns where a temporary buffer is allocated and
freed at the exit, and those can be simplified with the recent cleanup
mechanism via __free(kfree).

No functional changes, only code refactoring.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/seq_compat.c | 12 +++++-------
 sound/core/seq/seq_midi.c   | 14 +++-----------
 2 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/sound/core/seq/seq_compat.c b/sound/core/seq/seq_compat.c
index 1e35bf086a51..643af4c1e838 100644
--- a/sound/core/seq/seq_compat.c
+++ b/sound/core/seq/seq_compat.c
@@ -31,8 +31,8 @@ struct snd_seq_port_info32 {
 static int snd_seq_call_port_info_ioctl(struct snd_seq_client *client, unsigned int cmd,
 					struct snd_seq_port_info32 __user *data32)
 {
-	int err = -EFAULT;
-	struct snd_seq_port_info *data;
+	struct snd_seq_port_info *data __free(kfree) = NULL;
+	int err;
 
 	data = kmalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
@@ -41,20 +41,18 @@ static int snd_seq_call_port_info_ioctl(struct snd_seq_client *client, unsigned
 	if (copy_from_user(data, data32, sizeof(*data32)) ||
 	    get_user(data->flags, &data32->flags) ||
 	    get_user(data->time_queue, &data32->time_queue))
-		goto error;
+		return -EFAULT;
 	data->kernel = NULL;
 
 	err = snd_seq_kernel_client_ctl(client->number, cmd, data);
 	if (err < 0)
-		goto error;
+		return err;
 
 	if (copy_to_user(data32, data, sizeof(*data32)) ||
 	    put_user(data->flags, &data32->flags) ||
 	    put_user(data->time_queue, &data32->time_queue))
-		err = -EFAULT;
+		return -EFAULT;
 
- error:
-	kfree(data);
 	return err;
 }
 
diff --git a/sound/core/seq/seq_midi.c b/sound/core/seq/seq_midi.c
index 78dcb0ea1558..0594269d92ab 100644
--- a/sound/core/seq/seq_midi.c
+++ b/sound/core/seq/seq_midi.c
@@ -270,8 +270,8 @@ snd_seq_midisynth_probe(struct device *_dev)
 	struct snd_seq_device *dev = to_seq_dev(_dev);
 	struct seq_midisynth_client *client;
 	struct seq_midisynth *msynth, *ms;
-	struct snd_seq_port_info *port;
-	struct snd_rawmidi_info *info;
+	struct snd_seq_port_info *port __free(kfree) = NULL;
+	struct snd_rawmidi_info *info __free(kfree) = NULL;
 	struct snd_rawmidi *rmidi = dev->private_data;
 	int newclient = 0;
 	unsigned int p, ports;
@@ -297,10 +297,8 @@ snd_seq_midisynth_probe(struct device *_dev)
 	ports = output_count;
 	if (ports < input_count)
 		ports = input_count;
-	if (ports == 0) {
-		kfree(info);
+	if (ports == 0)
 		return -ENODEV;
-	}
 	if (ports > (256 / SNDRV_RAWMIDI_DEVICES))
 		ports = 256 / SNDRV_RAWMIDI_DEVICES;
 
@@ -311,7 +309,6 @@ snd_seq_midisynth_probe(struct device *_dev)
 		client = kzalloc(sizeof(*client), GFP_KERNEL);
 		if (client == NULL) {
 			mutex_unlock(&register_mutex);
-			kfree(info);
 			return -ENOMEM;
 		}
 		client->seq_client =
@@ -321,7 +318,6 @@ snd_seq_midisynth_probe(struct device *_dev)
 		if (client->seq_client < 0) {
 			kfree(client);
 			mutex_unlock(&register_mutex);
-			kfree(info);
 			return -ENOMEM;
 		}
 	}
@@ -403,8 +399,6 @@ snd_seq_midisynth_probe(struct device *_dev)
 	if (newclient)
 		synths[card->number] = client;
 	mutex_unlock(&register_mutex);
-	kfree(info);
-	kfree(port);
 	return 0;	/* success */
 
       __nomem:
@@ -417,8 +411,6 @@ snd_seq_midisynth_probe(struct device *_dev)
 		snd_seq_delete_kernel_client(client->seq_client);
 		kfree(client);
 	}
-	kfree(info);
-	kfree(port);
 	mutex_unlock(&register_mutex);
 	return -ENOMEM;
 }
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-02-22 11:15 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-22 11:15 [PATCH 0/9] ALSA: Use automatic cleanup of kfree() Takashi Iwai
2024-02-22 11:15 ` [PATCH 1/9] ALSA: pcm: " Takashi Iwai
2024-02-22 11:15 ` [PATCH 2/9] ALSA: control: " Takashi Iwai
2024-02-22 11:15 ` [PATCH 3/9] ALSA: compress_offload: " Takashi Iwai
2024-02-22 11:15 ` [PATCH 4/9] ALSA: timer: " Takashi Iwai
2024-02-22 11:15 ` [PATCH 5/9] ALSA: vmaster: " Takashi Iwai
2024-02-22 11:15 ` [PATCH 6/9] ALSA: seq: oss: " Takashi Iwai
2024-02-22 11:15 ` [PATCH 7/9] ALSA: seq: virmidi: " Takashi Iwai
2024-02-22 11:15 ` [PATCH 8/9] ALSA: seq: ump: " Takashi Iwai
2024-02-22 11:15 ` [PATCH 9/9] ALSA: seq: core: " Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox