Linux Sound subsystem development
 help / color / mirror / Atom feed
* [PATCH 0/3] ALSA: core: another small set of automatic cleanups
@ 2024-02-23  8:42 Takashi Iwai
  2024-02-23  8:42 ` [PATCH 1/3] ALSA: pcm: Use CLASS() for fdget()/fdput() Takashi Iwai
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Takashi Iwai @ 2024-02-23  8:42 UTC (permalink / raw)
  To: linux-sound

Hi,

here are the rest of small cleanups of ALSA core using the automatic
kfree and co.

Only minor code refactoring.


Takashi

===

Takashi Iwai (3):
  ALSA: pcm: Use CLASS() for fdget()/fdput()
  ALSA: mixer_oss: ump: Use automatic cleanup of kfree()
  ALSA: pcm_oss: ump: Use automatic cleanup of kfree()

 sound/core/oss/mixer_oss.c | 59 ++++++++++++--------------------------
 sound/core/oss/pcm_oss.c   | 33 +++++++--------------
 sound/core/pcm_native.c    | 22 +++++---------
 3 files changed, 35 insertions(+), 79 deletions(-)

-- 
2.35.3


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

* [PATCH 1/3] ALSA: pcm: Use CLASS() for fdget()/fdput()
  2024-02-23  8:42 [PATCH 0/3] ALSA: core: another small set of automatic cleanups Takashi Iwai
@ 2024-02-23  8:42 ` Takashi Iwai
  2024-02-23  8:42 ` [PATCH] ALSA: seq: prioq: Unify cell removal functions Takashi Iwai
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2024-02-23  8:42 UTC (permalink / raw)
  To: linux-sound

Now we have a nice definition of CLASS(fd) that can be applied as a
clean up for the fdget/fdput pairs in snd_pcm_link().

No functional changes, only code refactoring.

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

diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index beee5249dae1..0e84de4b484d 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2265,28 +2265,22 @@ static int snd_pcm_link(struct snd_pcm_substream *substream, int fd)
 	struct snd_pcm_group *group __free(kfree) = NULL;
 	struct snd_pcm_group *target_group;
 	bool nonatomic = substream->pcm->nonatomic;
-	struct fd f = fdget(fd);
+	CLASS(fd, f)(fd);
 
 	if (!f.file)
 		return -EBADFD;
-	if (!is_pcm_file(f.file)) {
-		res = -EBADFD;
-		goto _badf;
-	}
+	if (!is_pcm_file(f.file))
+		return -EBADFD;
 
 	pcm_file = f.file->private_data;
 	substream1 = pcm_file->substream;
 
-	if (substream == substream1) {
-		res = -EINVAL;
-		goto _badf;
-	}
+	if (substream == substream1)
+		return -EINVAL;
 
 	group = kzalloc(sizeof(*group), GFP_KERNEL);
-	if (!group) {
-		res = -ENOMEM;
-		goto _badf;
-	}
+	if (!group)
+		return -ENOMEM;
 
 	snd_pcm_group_init(group);
 
@@ -2318,8 +2312,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);
- _badf:
-	fdput(f);
 	return res;
 }
 
-- 
2.35.3


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

* [PATCH] ALSA: seq: prioq: Unify cell removal functions
  2024-02-23  8:42 [PATCH 0/3] ALSA: core: another small set of automatic cleanups Takashi Iwai
  2024-02-23  8:42 ` [PATCH 1/3] ALSA: pcm: Use CLASS() for fdget()/fdput() Takashi Iwai
@ 2024-02-23  8:42 ` Takashi Iwai
  2024-02-23  8:42 ` [PATCH 2/3] ALSA: mixer_oss: ump: Use automatic cleanup of kfree() Takashi Iwai
  2024-02-23  8:42 ` [PATCH 3/3] ALSA: pcm_oss: " Takashi Iwai
  3 siblings, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2024-02-23  8:42 UTC (permalink / raw)
  To: linux-sound

Both snd_seq_prioq_remove_events() and snd_seq_prioq_leave() have a
very similar loop for removing events.  Unify them with a callback for
code simplification.

Only the code refactoring, and no functional changes.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/seq_prioq.c | 197 ++++++++++++++++---------------------
 1 file changed, 84 insertions(+), 113 deletions(-)

diff --git a/sound/core/seq/seq_prioq.c b/sound/core/seq/seq_prioq.c
index 1d857981e876..e062df7610e1 100644
--- a/sound/core/seq/seq_prioq.c
+++ b/sound/core/seq/seq_prioq.c
@@ -249,30 +249,11 @@ int snd_seq_prioq_avail(struct snd_seq_prioq * f)
 	return f->cells;
 }
 
-static inline int prioq_match(struct snd_seq_event_cell *cell,
-			      int client, int timestamp)
-{
-	if (cell->event.source.client == client ||
-	    cell->event.dest.client == client)
-		return 1;
-	if (!timestamp)
-		return 0;
-	switch (cell->event.flags & SNDRV_SEQ_TIME_STAMP_MASK) {
-	case SNDRV_SEQ_TIME_STAMP_TICK:
-		if (cell->event.time.tick)
-			return 1;
-		break;
-	case SNDRV_SEQ_TIME_STAMP_REAL:
-		if (cell->event.time.time.tv_sec ||
-		    cell->event.time.time.tv_nsec)
-			return 1;
-		break;
-	}
-	return 0;
-}
-
-/* remove cells for left client */
-void snd_seq_prioq_leave(struct snd_seq_prioq * f, int client, int timestamp)
+/* remove cells matching with the condition */
+static void prioq_remove_cells(struct snd_seq_prioq *f,
+			       bool (*match)(struct snd_seq_event_cell *cell,
+					     void *arg),
+			       void *arg)
 {
 	register struct snd_seq_event_cell *cell, *next;
 	unsigned long flags;
@@ -281,39 +262,29 @@ void snd_seq_prioq_leave(struct snd_seq_prioq * f, int client, int timestamp)
 
 	/* collect all removed cells */
 	spin_lock_irqsave(&f->lock, flags);
-	cell = f->head;
-	while (cell) {
+	for (cell = f->head; cell; cell = next) {
 		next = cell->next;
-		if (prioq_match(cell, client, timestamp)) {
-			/* remove cell from prioq */
-			if (cell == f->head) {
-				f->head = cell->next;
-			} else {
-				prev->next = cell->next;
-			}
-			if (cell == f->tail)
-				f->tail = cell->next;
-			f->cells--;
-			/* add cell to free list */
-			cell->next = NULL;
-			if (freefirst == NULL) {
-				freefirst = cell;
-			} else {
-				freeprev->next = cell;
-			}
-			freeprev = cell;
-		} else {
-#if 0
-			pr_debug("ALSA: seq: type = %i, source = %i, dest = %i, "
-			       "client = %i\n",
-				cell->event.type,
-				cell->event.source.client,
-				cell->event.dest.client,
-				client);
-#endif
+		if (!match(cell, arg)) {
 			prev = cell;
+			continue;
 		}
-		cell = next;		
+
+		/* remove cell from prioq */
+		if (cell == f->head)
+			f->head = cell->next;
+		else
+			prev->next = cell->next;
+		if (cell == f->tail)
+			f->tail = cell->next;
+		f->cells--;
+
+		/* add cell to free list */
+		cell->next = NULL;
+		if (freefirst == NULL)
+			freefirst = cell;
+		else
+			freeprev->next = cell;
+		freeprev = cell;
 	}
 	spin_unlock_irqrestore(&f->lock, flags);	
 
@@ -325,22 +296,68 @@ void snd_seq_prioq_leave(struct snd_seq_prioq * f, int client, int timestamp)
 	}
 }
 
-static int prioq_remove_match(struct snd_seq_remove_events *info,
-			      struct snd_seq_event *ev)
+struct prioq_match_arg {
+	int client;
+	int timestamp;
+};
+
+static inline bool prioq_match(struct snd_seq_event_cell *cell, void *arg)
 {
+	struct prioq_match_arg *v = arg;
+
+	if (cell->event.source.client == v->client ||
+	    cell->event.dest.client == v->client)
+		return true;
+	if (!v->timestamp)
+		return false;
+	switch (cell->event.flags & SNDRV_SEQ_TIME_STAMP_MASK) {
+	case SNDRV_SEQ_TIME_STAMP_TICK:
+		if (cell->event.time.tick)
+			return true;
+		break;
+	case SNDRV_SEQ_TIME_STAMP_REAL:
+		if (cell->event.time.time.tv_sec ||
+		    cell->event.time.time.tv_nsec)
+			return true;
+		break;
+	}
+	return false;
+}
+
+/* remove cells for left client */
+void snd_seq_prioq_leave(struct snd_seq_prioq *f, int client, int timestamp)
+{
+	struct prioq_match_arg arg = { client, timestamp };
+
+	return prioq_remove_cells(f, prioq_match, &arg);
+}
+
+struct prioq_remove_match_arg {
+	int client;
+	struct snd_seq_remove_events *info;
+};
+
+static bool prioq_remove_match(struct snd_seq_event_cell *cell, void *arg)
+{
+	struct prioq_remove_match_arg *v = arg;
+	struct snd_seq_event *ev = &cell->event;
+	struct snd_seq_remove_events *info = v->info;
 	int res;
 
+	if (ev->source.client != v->client)
+		return false;
+
 	if (info->remove_mode & SNDRV_SEQ_REMOVE_DEST) {
 		if (ev->dest.client != info->dest.client ||
 				ev->dest.port != info->dest.port)
-			return 0;
+			return false;
 	}
 	if (info->remove_mode & SNDRV_SEQ_REMOVE_DEST_CHANNEL) {
 		if (! snd_seq_ev_is_channel_type(ev))
-			return 0;
+			return false;
 		/* data.note.channel and data.control.channel are identical */
 		if (ev->data.note.channel != info->channel)
-			return 0;
+			return false;
 	}
 	if (info->remove_mode & SNDRV_SEQ_REMOVE_TIME_AFTER) {
 		if (info->remove_mode & SNDRV_SEQ_REMOVE_TIME_TICK)
@@ -348,7 +365,7 @@ static int prioq_remove_match(struct snd_seq_remove_events *info,
 		else
 			res = snd_seq_compare_real_time(&ev->time.time, &info->time.time);
 		if (!res)
-			return 0;
+			return false;
 	}
 	if (info->remove_mode & SNDRV_SEQ_REMOVE_TIME_BEFORE) {
 		if (info->remove_mode & SNDRV_SEQ_REMOVE_TIME_TICK)
@@ -356,81 +373,35 @@ static int prioq_remove_match(struct snd_seq_remove_events *info,
 		else
 			res = snd_seq_compare_real_time(&ev->time.time, &info->time.time);
 		if (res)
-			return 0;
+			return false;
 	}
 	if (info->remove_mode & SNDRV_SEQ_REMOVE_EVENT_TYPE) {
 		if (ev->type != info->type)
-			return 0;
+			return false;
 	}
 	if (info->remove_mode & SNDRV_SEQ_REMOVE_IGNORE_OFF) {
 		/* Do not remove off events */
 		switch (ev->type) {
 		case SNDRV_SEQ_EVENT_NOTEOFF:
 		/* case SNDRV_SEQ_EVENT_SAMPLE_STOP: */
-			return 0;
+			return false;
 		default:
 			break;
 		}
 	}
 	if (info->remove_mode & SNDRV_SEQ_REMOVE_TAG_MATCH) {
 		if (info->tag != ev->tag)
-			return 0;
+			return false;
 	}
 
-	return 1;
+	return true;
 }
 
 /* remove cells matching remove criteria */
 void snd_seq_prioq_remove_events(struct snd_seq_prioq * f, int client,
 				 struct snd_seq_remove_events *info)
 {
-	struct snd_seq_event_cell *cell, *next;
-	unsigned long flags;
-	struct snd_seq_event_cell *prev = NULL;
-	struct snd_seq_event_cell *freefirst = NULL, *freeprev = NULL, *freenext;
+	struct prioq_remove_match_arg arg = { client, info };
 
-	/* collect all removed cells */
-	spin_lock_irqsave(&f->lock, flags);
-	cell = f->head;
-
-	while (cell) {
-		next = cell->next;
-		if (cell->event.source.client == client &&
-			prioq_remove_match(info, &cell->event)) {
-
-			/* remove cell from prioq */
-			if (cell == f->head) {
-				f->head = cell->next;
-			} else {
-				prev->next = cell->next;
-			}
-
-			if (cell == f->tail)
-				f->tail = cell->next;
-			f->cells--;
-
-			/* add cell to free list */
-			cell->next = NULL;
-			if (freefirst == NULL) {
-				freefirst = cell;
-			} else {
-				freeprev->next = cell;
-			}
-
-			freeprev = cell;
-		} else {
-			prev = cell;
-		}
-		cell = next;		
-	}
-	spin_unlock_irqrestore(&f->lock, flags);	
-
-	/* remove selected cells */
-	while (freefirst) {
-		freenext = freefirst->next;
-		snd_seq_cell_free(freefirst);
-		freefirst = freenext;
-	}
+	return prioq_remove_cells(f, prioq_remove_match, &arg);
 }
-
-
-- 
2.35.3


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

* [PATCH 2/3] ALSA: mixer_oss: ump: Use automatic cleanup of kfree()
  2024-02-23  8:42 [PATCH 0/3] ALSA: core: another small set of automatic cleanups Takashi Iwai
  2024-02-23  8:42 ` [PATCH 1/3] ALSA: pcm: Use CLASS() for fdget()/fdput() Takashi Iwai
  2024-02-23  8:42 ` [PATCH] ALSA: seq: prioq: Unify cell removal functions Takashi Iwai
@ 2024-02-23  8:42 ` Takashi Iwai
  2024-02-23  8:42 ` [PATCH 3/3] ALSA: pcm_oss: " Takashi Iwai
  3 siblings, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2024-02-23  8:42 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/oss/mixer_oss.c | 59 ++++++++++++--------------------------
 1 file changed, 18 insertions(+), 41 deletions(-)

diff --git a/sound/core/oss/mixer_oss.c b/sound/core/oss/mixer_oss.c
index dae2da380835..e10017a42ed8 100644
--- a/sound/core/oss/mixer_oss.c
+++ b/sound/core/oss/mixer_oss.c
@@ -532,8 +532,8 @@ static void snd_mixer_oss_get_volume1_vol(struct snd_mixer_oss_file *fmixer,
 					  unsigned int numid,
 					  int *left, int *right)
 {
-	struct snd_ctl_elem_info *uinfo;
-	struct snd_ctl_elem_value *uctl;
+	struct snd_ctl_elem_info *uinfo __free(kfree) = NULL;
+	struct snd_ctl_elem_value *uctl __free(kfree) = NULL;
 	struct snd_kcontrol *kctl;
 	struct snd_card *card = fmixer->card;
 
@@ -561,8 +561,6 @@ static void snd_mixer_oss_get_volume1_vol(struct snd_mixer_oss_file *fmixer,
 		*right = snd_mixer_oss_conv1(uctl->value.integer.value[1], uinfo->value.integer.min, uinfo->value.integer.max, &pslot->volume[1]);
       __unalloc:
 	up_read(&card->controls_rwsem);
-      	kfree(uctl);
-      	kfree(uinfo);
 }
 
 static void snd_mixer_oss_get_volume1_sw(struct snd_mixer_oss_file *fmixer,
@@ -571,8 +569,8 @@ static void snd_mixer_oss_get_volume1_sw(struct snd_mixer_oss_file *fmixer,
 					 int *left, int *right,
 					 int route)
 {
-	struct snd_ctl_elem_info *uinfo;
-	struct snd_ctl_elem_value *uctl;
+	struct snd_ctl_elem_info *uinfo __free(kfree) = NULL;
+	struct snd_ctl_elem_value *uctl __free(kfree) = NULL;
 	struct snd_kcontrol *kctl;
 	struct snd_card *card = fmixer->card;
 
@@ -601,8 +599,6 @@ static void snd_mixer_oss_get_volume1_sw(struct snd_mixer_oss_file *fmixer,
 		*right = 0;
       __unalloc:
 	up_read(&card->controls_rwsem);
-      	kfree(uctl);
-	kfree(uinfo);
 }
 
 static int snd_mixer_oss_get_volume1(struct snd_mixer_oss_file *fmixer,
@@ -636,8 +632,8 @@ static void snd_mixer_oss_put_volume1_vol(struct snd_mixer_oss_file *fmixer,
 					  unsigned int numid,
 					  int left, int right)
 {
-	struct snd_ctl_elem_info *uinfo;
-	struct snd_ctl_elem_value *uctl;
+	struct snd_ctl_elem_info *uinfo __free(kfree) = NULL;
+	struct snd_ctl_elem_value *uctl __free(kfree) = NULL;
 	struct snd_kcontrol *kctl;
 	struct snd_card *card = fmixer->card;
 	int res;
@@ -669,8 +665,6 @@ static void snd_mixer_oss_put_volume1_vol(struct snd_mixer_oss_file *fmixer,
 		snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_VALUE, &kctl->id);
       __unalloc:
 	up_read(&card->controls_rwsem);
-      	kfree(uctl);
-	kfree(uinfo);
 }
 
 static void snd_mixer_oss_put_volume1_sw(struct snd_mixer_oss_file *fmixer,
@@ -679,8 +673,8 @@ static void snd_mixer_oss_put_volume1_sw(struct snd_mixer_oss_file *fmixer,
 					 int left, int right,
 					 int route)
 {
-	struct snd_ctl_elem_info *uinfo;
-	struct snd_ctl_elem_value *uctl;
+	struct snd_ctl_elem_info *uinfo __free(kfree) = NULL;
+	struct snd_ctl_elem_value *uctl __free(kfree) = NULL;
 	struct snd_kcontrol *kctl;
 	struct snd_card *card = fmixer->card;
 	int res;
@@ -716,8 +710,6 @@ static void snd_mixer_oss_put_volume1_sw(struct snd_mixer_oss_file *fmixer,
 		snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_VALUE, &kctl->id);
       __unalloc:
 	up_read(&card->controls_rwsem);
-      	kfree(uctl);
-	kfree(uinfo);
 }
 
 static int snd_mixer_oss_put_volume1(struct snd_mixer_oss_file *fmixer,
@@ -822,16 +814,14 @@ static int snd_mixer_oss_get_recsrc2(struct snd_mixer_oss_file *fmixer, unsigned
 	struct snd_kcontrol *kctl;
 	struct snd_mixer_oss_slot *pslot;
 	struct slot *slot;
-	struct snd_ctl_elem_info *uinfo;
-	struct snd_ctl_elem_value *uctl;
+	struct snd_ctl_elem_info *uinfo __free(kfree) = NULL;
+	struct snd_ctl_elem_value *uctl __free(kfree) = NULL;
 	int err, idx;
 	
 	uinfo = kzalloc(sizeof(*uinfo), GFP_KERNEL);
 	uctl = kzalloc(sizeof(*uctl), GFP_KERNEL);
-	if (uinfo == NULL || uctl == NULL) {
-		err = -ENOMEM;
-		goto __free_only;
-	}
+	if (uinfo == NULL || uctl == NULL)
+		return -ENOMEM;
 	down_read(&card->controls_rwsem);
 	kctl = snd_mixer_oss_test_id(mixer, "Capture Source", 0);
 	if (! kctl) {
@@ -861,9 +851,6 @@ static int snd_mixer_oss_get_recsrc2(struct snd_mixer_oss_file *fmixer, unsigned
 	err = 0;
       __unlock:
      	up_read(&card->controls_rwsem);
-      __free_only:
-      	kfree(uctl);
-      	kfree(uinfo);
       	return err;
 }
 
@@ -874,17 +861,15 @@ static int snd_mixer_oss_put_recsrc2(struct snd_mixer_oss_file *fmixer, unsigned
 	struct snd_kcontrol *kctl;
 	struct snd_mixer_oss_slot *pslot;
 	struct slot *slot = NULL;
-	struct snd_ctl_elem_info *uinfo;
-	struct snd_ctl_elem_value *uctl;
+	struct snd_ctl_elem_info *uinfo __free(kfree) = NULL;
+	struct snd_ctl_elem_value *uctl __free(kfree) = NULL;
 	int err;
 	unsigned int idx;
 
 	uinfo = kzalloc(sizeof(*uinfo), GFP_KERNEL);
 	uctl = kzalloc(sizeof(*uctl), GFP_KERNEL);
-	if (uinfo == NULL || uctl == NULL) {
-		err = -ENOMEM;
-		goto __free_only;
-	}
+	if (uinfo == NULL || uctl == NULL)
+		return -ENOMEM;
 	down_read(&card->controls_rwsem);
 	kctl = snd_mixer_oss_test_id(mixer, "Capture Source", 0);
 	if (! kctl) {
@@ -917,9 +902,6 @@ static int snd_mixer_oss_put_recsrc2(struct snd_mixer_oss_file *fmixer, unsigned
 	err = 0;
       __unlock:
 	up_read(&card->controls_rwsem);
-      __free_only:
-	kfree(uctl);
-	kfree(uinfo);
 	return err;
 }
 
@@ -931,7 +913,7 @@ struct snd_mixer_oss_assign_table {
 
 static int snd_mixer_oss_build_test(struct snd_mixer_oss *mixer, struct slot *slot, const char *name, int index, int item)
 {
-	struct snd_ctl_elem_info *info;
+	struct snd_ctl_elem_info *info __free(kfree) = NULL;
 	struct snd_kcontrol *kcontrol;
 	struct snd_card *card = mixer->card;
 	int err;
@@ -950,7 +932,6 @@ static int snd_mixer_oss_build_test(struct snd_mixer_oss *mixer, struct slot *sl
 	err = kcontrol->info(kcontrol, info);
 	if (err < 0) {
 		up_read(&card->controls_rwsem);
-		kfree(info);
 		return err;
 	}
 	slot->numid[item] = kcontrol->id.numid;
@@ -958,7 +939,6 @@ static int snd_mixer_oss_build_test(struct snd_mixer_oss *mixer, struct slot *sl
 	if (info->count > slot->channels)
 		slot->channels = info->count;
 	slot->present |= 1 << item;
-	kfree(info);
 	return 0;
 }
 
@@ -1073,7 +1053,7 @@ static int snd_mixer_oss_build_input(struct snd_mixer_oss *mixer,
 	if (!ptr->index)
 		kctl = snd_mixer_oss_test_id(mixer, "Capture Source", 0);
 	if (kctl) {
-		struct snd_ctl_elem_info *uinfo;
+		struct snd_ctl_elem_info *uinfo __free(kfree) = NULL;
 
 		uinfo = kzalloc(sizeof(*uinfo), GFP_KERNEL);
 		if (! uinfo) {
@@ -1083,7 +1063,6 @@ static int snd_mixer_oss_build_input(struct snd_mixer_oss *mixer,
 			
 		if (kctl->info(kctl, uinfo)) {
 			up_read(&mixer->card->controls_rwsem);
-			kfree(uinfo);
 			return 0;
 		}
 		strcpy(str, ptr->name);
@@ -1099,7 +1078,6 @@ static int snd_mixer_oss_build_input(struct snd_mixer_oss *mixer,
 				uinfo->value.enumerated.item = slot.capture_item;
 				if (kctl->info(kctl, uinfo)) {
 					up_read(&mixer->card->controls_rwsem);
-					kfree(uinfo);
 					return 0;
 				}
 				if (!strcmp(uinfo->value.enumerated.name, str)) {
@@ -1108,7 +1086,6 @@ static int snd_mixer_oss_build_input(struct snd_mixer_oss *mixer,
 				}
 			}
 		}
-		kfree(uinfo);
 	}
 	up_read(&mixer->card->controls_rwsem);
 	if (slot.present != 0) {
-- 
2.35.3


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

* [PATCH 3/3] ALSA: pcm_oss: ump: Use automatic cleanup of kfree()
  2024-02-23  8:42 [PATCH 0/3] ALSA: core: another small set of automatic cleanups Takashi Iwai
                   ` (2 preceding siblings ...)
  2024-02-23  8:42 ` [PATCH 2/3] ALSA: mixer_oss: ump: Use automatic cleanup of kfree() Takashi Iwai
@ 2024-02-23  8:42 ` Takashi Iwai
  3 siblings, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2024-02-23  8:42 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/oss/pcm_oss.c | 33 ++++++++++-----------------------
 1 file changed, 10 insertions(+), 23 deletions(-)

diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index 728c211142d1..e4e292b2db06 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -377,7 +377,7 @@ static int snd_pcm_hw_param_near(struct snd_pcm_substream *pcm,
 				 snd_pcm_hw_param_t var, unsigned int best,
 				 int *dir)
 {
-	struct snd_pcm_hw_params *save = NULL;
+	struct snd_pcm_hw_params *save __free(kfree) = NULL;
 	int v;
 	unsigned int saved_min;
 	int last = 0;
@@ -404,38 +404,30 @@ static int snd_pcm_hw_param_near(struct snd_pcm_substream *pcm,
 	saved_min = min;
 	min = snd_pcm_hw_param_min(pcm, params, var, min, &mindir);
 	if (min >= 0) {
-		struct snd_pcm_hw_params *params1;
+		struct snd_pcm_hw_params *params1 __free(kfree) = NULL;
 		if (max < 0)
 			goto _end;
 		if ((unsigned int)min == saved_min && mindir == valdir)
 			goto _end;
 		params1 = kmalloc(sizeof(*params1), GFP_KERNEL);
-		if (params1 == NULL) {
-			kfree(save);
+		if (params1 == NULL)
 			return -ENOMEM;
-		}
 		*params1 = *save;
 		max = snd_pcm_hw_param_max(pcm, params1, var, max, &maxdir);
-		if (max < 0) {
-			kfree(params1);
+		if (max < 0)
 			goto _end;
-		}
 		if (boundary_nearer(max, maxdir, best, valdir, min, mindir)) {
 			*params = *params1;
 			last = 1;
 		}
-		kfree(params1);
 	} else {
 		*params = *save;
 		max = snd_pcm_hw_param_max(pcm, params, var, max, &maxdir);
-		if (max < 0) {
-			kfree(save);
+		if (max < 0)
 			return max;
-		}
 		last = 1;
 	}
  _end:
- 	kfree(save);
 	if (last)
 		v = snd_pcm_hw_param_last(pcm, params, var, dir);
 	else
@@ -789,7 +781,7 @@ static int choose_rate(struct snd_pcm_substream *substream,
 		       struct snd_pcm_hw_params *params, unsigned int best_rate)
 {
 	const struct snd_interval *it;
-	struct snd_pcm_hw_params *save;
+	struct snd_pcm_hw_params *save __free(kfree) = NULL;
 	unsigned int rate, prev;
 
 	save = kmalloc(sizeof(*save), GFP_KERNEL);
@@ -808,10 +800,8 @@ static int choose_rate(struct snd_pcm_substream *substream,
 			ret = snd_pcm_hw_param_set(substream, params,
 						   SNDRV_PCM_HW_PARAM_RATE,
 						   rate, 0);
-			if (ret == (int)rate) {
-				kfree(save);
+			if (ret == (int)rate)
 				return rate;
-			}
 			*params = *save;
 		}
 		prev = rate;
@@ -821,7 +811,6 @@ static int choose_rate(struct snd_pcm_substream *substream,
 	}
 
 	/* not found, use the nearest rate */
-	kfree(save);
 	return snd_pcm_hw_param_near(substream, params, SNDRV_PCM_HW_PARAM_RATE, best_rate, NULL);
 }
 
@@ -1847,7 +1836,7 @@ static int snd_pcm_oss_get_formats(struct snd_pcm_oss_file *pcm_oss_file)
 	struct snd_pcm_substream *substream;
 	int err;
 	int direct;
-	struct snd_pcm_hw_params *params;
+	struct snd_pcm_hw_params *params __free(kfree) = NULL;
 	unsigned int formats = 0;
 	const struct snd_mask *format_mask;
 	int fmt;
@@ -1873,7 +1862,7 @@ static int snd_pcm_oss_get_formats(struct snd_pcm_oss_file *pcm_oss_file)
 	_snd_pcm_hw_params_any(params);
 	err = snd_pcm_hw_refine(substream, params);
 	if (err < 0)
-		goto error;
+		return err;
 	format_mask = hw_param_mask_c(params, SNDRV_PCM_HW_PARAM_FORMAT);
 	for (fmt = 0; fmt < 32; ++fmt) {
 		if (snd_mask_test(format_mask, fmt)) {
@@ -1883,9 +1872,7 @@ static int snd_pcm_oss_get_formats(struct snd_pcm_oss_file *pcm_oss_file)
 		}
 	}
 
- error:
-	kfree(params);
-	return err < 0 ? err : formats;
+	return formats;
 }
 
 static int snd_pcm_oss_set_format(struct snd_pcm_oss_file *pcm_oss_file, int format)
-- 
2.35.3


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

end of thread, other threads:[~2024-02-23  8:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-23  8:42 [PATCH 0/3] ALSA: core: another small set of automatic cleanups Takashi Iwai
2024-02-23  8:42 ` [PATCH 1/3] ALSA: pcm: Use CLASS() for fdget()/fdput() Takashi Iwai
2024-02-23  8:42 ` [PATCH] ALSA: seq: prioq: Unify cell removal functions Takashi Iwai
2024-02-23  8:42 ` [PATCH 2/3] ALSA: mixer_oss: ump: Use automatic cleanup of kfree() Takashi Iwai
2024-02-23  8:42 ` [PATCH 3/3] ALSA: pcm_oss: " Takashi Iwai

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