[PATCH] alsa-lib: snd_device_name_hint misbehaving

[PATCH] alsa-lib: snd_device_name_hint misbehaving

[John Lindgren]

* Remove erroneous snd_config_delete calls that cause later calls to
snd_pcm_open to fail.

* Don't list card-independent PCM devices ("null" or PulseAudio, for
example) more than once even if multiple sound cards are installed.

* Add "ctl" to the list of device types that snd_device_name_hint can
search for.

* Cache the path to libasound.so within snd_dlopen rather than looking
it up with dladdr every call.

For a longer explanation:
http://mailman.alsa-project.org/pipermail/alsa-devel/2009-October/022505.html

Signed-off-by: John Lindgren <john.lindgren@tds.net>

------

 src/conf.c             |    2 --
 src/control/namehint.c |   34 +++++++++++++++++++++++++++-------
 src/dlmisc.c           |   10 +++++++---
 3 files changed, 34 insertions(+), 12 deletions(-)

------

diff --git a/src/conf.c b/src/conf.c
index 570c90f..52a477c 100644
--- a/src/conf.c
+++ b/src/conf.c
@@ -1132,7 +1132,6 @@ static int parse_def(snd_config_t *parent, input_t *input, int skip, int overrid
 				free(id);
 				continue;
 			}
-			snd_config_delete(n);
 		}
 		if (mode == MERGE) {
 			SNDERR("%s does not exists", id);
@@ -1156,7 +1155,6 @@ static int parse_def(snd_config_t *parent, input_t *input, int skip, int overrid
 				skip = 1;
 				n = NULL;
 			} else if (mode == OVERRIDE) {
-				snd_config_delete(n);
 				n = NULL;
 			}
 		} else {
diff --git a/src/control/namehint.c b/src/control/namehint.c
index e878f83..ec9d04f 100644
--- a/src/control/namehint.c
+++ b/src/control/namehint.c
@@ -328,7 +328,6 @@ static int try_config(struct hint_list *list,
 	if (snd_config_search(cfg1, "slave", &cfg) >= 0 &&
 	    snd_config_search(cfg, base, &cfg1) >= 0)
 	    	goto __hint;
-	snd_config_delete(res);
 	res = NULL;
 	if (strchr(buf, ':') != NULL)
 		goto __ok;
@@ -379,8 +378,6 @@ static int try_config(struct hint_list *list,
 	      	err = hint_list_add(list, buf, buf1);
 	}
       __skip_add:
-      	if (res)
-	      	snd_config_delete(res);
 	if (buf1)
 		free(buf1);
       	free(buf);
@@ -450,13 +447,10 @@ static int add_card(struct hint_list *list, int card)
 		if (err == -EXDEV)
 			continue;
 		if (err < 0) {
+			list->card = card;
 			list->device = -1;
 			err = try_config(list, list->siface, str);
 		}
-		if (err < 0) {
-			list->card = -1;
-			err = try_config(list, list->siface, str);
-		}
 		if (err == -ENOMEM)
 			goto __error;
 	}
@@ -466,6 +460,29 @@ static int add_card(struct hint_list *list, int card)
 	return err;
 }
 
+static int add_software_devices(struct hint_list *list)
+{
+	int err;
+	snd_config_t *conf, *n;
+	snd_config_iterator_t i, next;
+	const char *str;
+
+	err = snd_config_search(snd_config, list->siface, &conf);
+	if (err < 0)
+		return err;
+	snd_config_for_each(i, next, conf) {
+		n = snd_config_iterator_entry(i);
+		if (snd_config_get_id(n, &str) < 0)
+			continue;
+		list->card = -1;
+		list->device = -1;
+		err = try_config(list, list->siface, str);
+		if (err == -ENOMEM)
+			return -ENOMEM;
+	}
+	return 0;
+}
+
 static int get_card_name(struct hint_list *list, int card)
 {
 	char scard[16], *s;
@@ -532,6 +549,8 @@ int snd_device_name_hint(int card, const char *iface, void ***hints)
 		list.iface = SND_CTL_ELEM_IFACE_SEQUENCER;
 	else if (strcmp(iface, "hwdep") == 0)
 		list.iface = SND_CTL_ELEM_IFACE_HWDEP;
+	else if (strcmp(iface, "ctl") == 0)
+		list.iface = SND_CTL_ELEM_IFACE_MIXER;
 	else
 		return -EINVAL;
 	list.show_all = 0;
@@ -543,6 +562,7 @@ int snd_device_name_hint(int card, const char *iface, void ***hints)
 		if (err >= 0)
 			err = add_card(&list, card);
 	} else {
+		add_software_devices(&list);
 		err = snd_card_next(&card);
 		if (err < 0)
 			goto __error;
diff --git a/src/dlmisc.c b/src/dlmisc.c
index c882cdc..3cca0f0 100644
--- a/src/dlmisc.c
+++ b/src/dlmisc.c
@@ -54,9 +54,13 @@ void *snd_dlopen(const char *name, int mode)
 #else
 #ifdef HAVE_LIBDL
 	if (name == NULL) {
-		Dl_info dlinfo;
-		if (dladdr(snd_dlopen, &dlinfo) > 0)
-			name = dlinfo.dli_fname;
+		static const char * self = NULL;
+		if (self == NULL) {
+			Dl_info dlinfo;
+			if (dladdr(snd_dlopen, &dlinfo) > 0)
+				self = dlinfo.dli_fname;
+		}
+		name = self;
 	}
 #endif
 #endif

Re: [PATCH] alsa-lib: snd_device_name_hint misbehaving

[Jaroslav Kysela]

On Sun, 1 Nov 2009, John Lindgren wrote:

> * Remove erroneous snd_config_delete calls that cause later calls to
> snd_pcm_open to fail.

It does not look good. Have you checked with valgrind if there are no 
memory leaks?

> * Don't list card-independent PCM devices ("null" or PulseAudio, for
> example) more than once even if multiple sound cards are installed.
>
> * Add "ctl" to the list of device types that snd_device_name_hint can
> search for.
>
> * Cache the path to libasound.so within snd_dlopen rather than looking
> it up with dladdr every call.

Please, split your changes to single patches. Thanks.

 					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: [PATCH] alsa-lib: snd_device_name_hint misbehaving

[John Lindgren]

On Mon, 2009-11-02 at 07:21 +0100, Jaroslav Kysela wrote:
> On Sun, 1 Nov 2009, John Lindgren wrote:
> 
> > * Remove erroneous snd_config_delete calls that cause later calls to
> > snd_pcm_open to fail.
> 
> It does not look good. Have you checked with valgrind if there are no 
> memory leaks?

You're making the same mistake that the original author of the code
made.  snd_config_delete is not a call to free memory; it is a call to
remove sections of the loaded configuration.  The configuration nodes
that were being passed to snd_config_delete were returned by
snd_config_search, which returns direct pointers to the actual nodes in
the configuration.  It does no memory duplication.

> Please, split your changes to single patches. Thanks.

I will do so when I have the time to learn Git a little better.  (I
normally use Mercurial).

John Lindgren

Re: [PATCH] alsa-lib: snd_device_name_hint misbehaving

[Takashi Iwai]

At Mon, 02 Nov 2009 13:12:21 -0500,
John Lindgren wrote:
> 
> On Mon, 2009-11-02 at 07:21 +0100, Jaroslav Kysela wrote:
> > On Sun, 1 Nov 2009, John Lindgren wrote:
> > 
> > > * Remove erroneous snd_config_delete calls that cause later calls to
> > > snd_pcm_open to fail.
> > 
> > It does not look good. Have you checked with valgrind if there are no 
> > memory leaks?
> 
> You're making the same mistake that the original author of the code
> made.  snd_config_delete is not a call to free memory; it is a call to
> remove sections of the loaded configuration.  The configuration nodes
> that were being passed to snd_config_delete were returned by
> snd_config_search, which returns direct pointers to the actual nodes in
> the configuration.  It does no memory duplication.

Well, no, the code there should be OK.  It's the place to replace the
whole sub-tree by overriding the object.  Thus, freeing the object
returned from snd_config_search() is correct.

The problem is rather in namehint.c, as found in my patch in another
post.

> > Please, split your changes to single patches. Thanks.
> 
> I will do so when I have the time to learn Git a little better.  (I
> normally use Mercurial).

This is easy even without VCS.  Just split your own patch to several
files by an editor :)   Namely, the change of dlmisc.c, and the
addition of "ctl" check in namehint.c.


thanks,

Takashi

Re: [PATCH] alsa-lib: snd_device_name_hint misbehaving

[Raymond Yau]

Is there any test program for this change since currently only "hw" and
"pulse" can be listed by name hint ?

* Add "ctl" to the list of device types that snd_device_name_hint can
search for.



2009/11/2 John Lindgren <john.lindgren@tds.net>

> * Remove erroneous snd_config_delete calls that cause later calls to
> snd_pcm_open to fail.
>
> * Don't list card-independent PCM devices ("null" or PulseAudio, for
> example) more than once even if multiple sound cards are installed.
>
> * Add "ctl" to the list of device types that snd_device_name_hint can
> search for.
>
> * Cache the path to libasound.so within snd_dlopen rather than looking
> it up with dladdr every call.
>
> For a longer explanation:
>
> http://mailman.alsa-project.org/pipermail/alsa-devel/2009-October/022505.html
>
> Signed-off-by: John Lindgren <john.lindgren@tds.net>
>
> ------
>
>  src/conf.c             |    2 --
>  src/control/namehint.c |   34 +++++++++++++++++++++++++++-------
>  src/dlmisc.c           |   10 +++++++---
>  3 files changed, 34 insertions(+), 12 deletions(-)
>
> ------
>
> diff --git a/src/conf.c b/src/conf.c
> index 570c90f..52a477c 100644
> --- a/src/conf.c
> +++ b/src/conf.c
> @@ -1132,7 +1132,6 @@ static int parse_def(snd_config_t *parent, input_t
> *input, int skip, int overrid
>                                free(id);
>                                continue;
>                        }
> -                       snd_config_delete(n);
>                }
>                if (mode == MERGE) {
>                        SNDERR("%s does not exists", id);
> @@ -1156,7 +1155,6 @@ static int parse_def(snd_config_t *parent, input_t
> *input, int skip, int overrid
>                                skip = 1;
>                                n = NULL;
>                        } else if (mode == OVERRIDE) {
> -                               snd_config_delete(n);
>                                n = NULL;
>                        }
>                } else {
> diff --git a/src/control/namehint.c b/src/control/namehint.c
> index e878f83..ec9d04f 100644
> --- a/src/control/namehint.c
> +++ b/src/control/namehint.c
> @@ -328,7 +328,6 @@ static int try_config(struct hint_list *list,
>        if (snd_config_search(cfg1, "slave", &cfg) >= 0 &&
>            snd_config_search(cfg, base, &cfg1) >= 0)
>                goto __hint;
> -       snd_config_delete(res);
>        res = NULL;
>        if (strchr(buf, ':') != NULL)
>                goto __ok;
> @@ -379,8 +378,6 @@ static int try_config(struct hint_list *list,
>                err = hint_list_add(list, buf, buf1);
>        }
>       __skip_add:
> -       if (res)
> -               snd_config_delete(res);
>        if (buf1)
>                free(buf1);
>        free(buf);
> @@ -450,13 +447,10 @@ static int add_card(struct hint_list *list, int card)
>                if (err == -EXDEV)
>                        continue;
>                if (err < 0) {
> +                       list->card = card;
>                        list->device = -1;
>                        err = try_config(list, list->siface, str);
>                }
> -               if (err < 0) {
> -                       list->card = -1;
> -                       err = try_config(list, list->siface, str);
> -               }
>                if (err == -ENOMEM)
>                        goto __error;
>        }
> @@ -466,6 +460,29 @@ static int add_card(struct hint_list *list, int card)
>        return err;
>  }
>
> +static int add_software_devices(struct hint_list *list)
> +{
> +       int err;
> +       snd_config_t *conf, *n;
> +       snd_config_iterator_t i, next;
> +       const char *str;
> +
> +       err = snd_config_search(snd_config, list->siface, &conf);
> +       if (err < 0)
> +               return err;
> +       snd_config_for_each(i, next, conf) {
> +               n = snd_config_iterator_entry(i);
> +               if (snd_config_get_id(n, &str) < 0)
> +                       continue;
> +               list->card = -1;
> +               list->device = -1;
> +               err = try_config(list, list->siface, str);
> +               if (err == -ENOMEM)
> +                       return -ENOMEM;
> +       }
> +       return 0;
> +}
> +
>  static int get_card_name(struct hint_list *list, int card)
>  {
>        char scard[16], *s;
> @@ -532,6 +549,8 @@ int snd_device_name_hint(int card, const char *iface,
> void ***hints)
>                list.iface = SND_CTL_ELEM_IFACE_SEQUENCER;
>        else if (strcmp(iface, "hwdep") == 0)
>                list.iface = SND_CTL_ELEM_IFACE_HWDEP;
> +       else if (strcmp(iface, "ctl") == 0)
> +               list.iface = SND_CTL_ELEM_IFACE_MIXER;
>        else
>                return -EINVAL;
>        list.show_all = 0;
> @@ -543,6 +562,7 @@ int snd_device_name_hint(int card, const char *iface,
> void ***hints)
>                if (err >= 0)
>                        err = add_card(&list, card);
>        } else {
> +               add_software_devices(&list);
>                err = snd_card_next(&card);
>                if (err < 0)
>                        goto __error;
> diff --git a/src/dlmisc.c b/src/dlmisc.c
> index c882cdc..3cca0f0 100644
> --- a/src/dlmisc.c
> +++ b/src/dlmisc.c
> @@ -54,9 +54,13 @@ void *snd_dlopen(const char *name, int mode)
>  #else
>  #ifdef HAVE_LIBDL
>        if (name == NULL) {
> -               Dl_info dlinfo;
> -               if (dladdr(snd_dlopen, &dlinfo) > 0)
> -                       name = dlinfo.dli_fname;
> +               static const char * self = NULL;
> +               if (self == NULL) {
> +                       Dl_info dlinfo;
> +                       if (dladdr(snd_dlopen, &dlinfo) > 0)
> +                               self = dlinfo.dli_fname;
> +               }
> +               name = self;
>        }
>  #endif
>  #endif
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>

Re: [PATCH] alsa-lib: snd_device_name_hint misbehaving

[Takashi Iwai]

At Sun, 01 Nov 2009 21:23:06 -0500,
John Lindgren wrote:
> 
> * Remove erroneous snd_config_delete calls that cause later calls to
> snd_pcm_open to fail.

This is bad indeed.  Could you make a small test case to reproduce the
problem more easily?


thanks,

Takashi

Re: [PATCH] alsa-lib: snd_device_name_hint misbehaving

[John Lindgren]

On Mon, 2009-11-02 at 14:08 +0100, Takashi Iwai wrote:
> > * Remove erroneous snd_config_delete calls that cause later calls to
> > snd_pcm_open to fail.
> 
> This is bad indeed.  Could you make a small test case to reproduce the
> problem more easily?

Sure.

------

#include <stdio.h>
#include <alsa/asoundlib.h>

void try_open (const char * pcm)
{
    snd_pcm_t * handle;
    int error;

    error = snd_pcm_open (& handle, pcm, SND_PCM_STREAM_PLAYBACK, 0);

    if (error < 0)
        printf ("Failed to open %s: %s.\n", pcm, snd_strerror (error));
    else
    {
        printf ("Opened %s.\n", pcm);
        snd_pcm_close (handle);
    }
}

int main (void)
{
    void * * hints;

    try_open ("default");
    try_open ("front");
    try_open ("pulse");
    try_open ("default");
    try_open ("front");
    try_open ("pulse");

    printf ("Calling snd_device_name_hint().\n");
    snd_device_name_hint (-1, "pcm", & hints);
    snd_device_name_free_hint (hints);

    try_open ("default");
    try_open ("front");
    try_open ("pulse");
    try_open ("default");
    try_open ("front");
    try_open ("pulse");

    return 0;
}

------

$ ./pcmtest
Opened default.
Opened front.
Opened pulse.
Opened default.
Opened front.
Opened pulse.
Calling snd_device_name_hint().
Opened default.
ALSA lib pcm.c:2211:(snd_pcm_open_noupdate) Unknown PCM cards.pcm.front
Failed to open front: No such file or directory.
ALSA lib pcm.c:2211:(snd_pcm_open_noupdate) Unknown PCM pulse
Failed to open pulse: No such file or directory.
Opened default.
ALSA lib pcm.c:2211:(snd_pcm_open_noupdate) Unknown PCM cards.pcm.front
Failed to open front: No such file or directory.
ALSA lib pcm.c:2211:(snd_pcm_open_noupdate) Unknown PCM pulse
Failed to open pulse: No such file or directory.

Re: [PATCH] alsa-lib: snd_device_name_hint misbehaving

[Takashi Iwai]

At Mon, 02 Nov 2009 09:43:14 -0500,
John Lindgren wrote:
> 
> On Mon, 2009-11-02 at 14:08 +0100, Takashi Iwai wrote:
> > > * Remove erroneous snd_config_delete calls that cause later calls to
> > > snd_pcm_open to fail.
> > 
> > This is bad indeed.  Could you make a small test case to reproduce the
> > problem more easily?
> 
> Sure.

Thanks.  I guess this depends on the config files.
Could you attach your ones?


Takashi

Re: [PATCH] alsa-lib: snd_device_name_hint misbehaving

[John Lindgren]

[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]

On Mon, 2009-11-02 at 15:55 +0100, Takashi Iwai wrote:
> Thanks.  I guess this depends on the config files.
> Could you attach your ones?

I can reproduce with only the stock /usr/share/alsa/alsa.conf from
Debian installed if I try to use the "null" device after
snd_device_name_hint.  /usr/share/alsa/alsa.conf is attached.

------

#include <stdio.h>
#include <alsa/asoundlib.h>

void try_open (const char * pcm)
{
    snd_pcm_t * handle;
    int error;

    error = snd_pcm_open (& handle, pcm, SND_PCM_STREAM_PLAYBACK, 0);

    if (error < 0)
        printf ("Failed to open %s: %s.\n", pcm, snd_strerror (error));
    else
    {
        printf ("Opened %s.\n", pcm);
        snd_pcm_close (handle);
    }
}

int main (void)
{
    void * * hints;

    try_open ("default");
    try_open ("null");
    try_open ("default");
    try_open ("null");

    printf ("Calling snd_device_name_hint().\n");
    snd_device_name_hint (-1, "pcm", & hints);
    snd_device_name_free_hint (hints);

    try_open ("default");
    try_open ("null");
    try_open ("default");
    try_open ("null");

    return 0;
}

------

$ ./pcmtest 
Opened default.
Opened null.
Opened default.
Opened null.
Calling snd_device_name_hint().
Opened default.
ALSA lib pcm.c:2211:(snd_pcm_open_noupdate) Unknown PCM null
Failed to open null: No such file or directory.
Opened default.
ALSA lib pcm.c:2211:(snd_pcm_open_noupdate) Unknown PCM null
Failed to open null: No such file or directory.

[-- Attachment #2: alsa.conf --]
[-- Type: text/plain, Size: 8967 bytes --]

#
#  ALSA library configuration file
#

# pre-load the configuration files

@hooks [
	{
		func load
		files [
			"/etc/asound.conf"
			"~/.asoundrc"
		]
		errors false
	}
]

# load card-specific configuration files (on request)

cards.@hooks [
	{
		func load
		files [
			{
				@func concat
				strings [
					{ @func datadir }
					"/cards/aliases.conf"
				]
			}
		]
	}
	{
		func load_for_all_cards
		files [
			{
				@func concat
				strings [
					{ @func datadir }
					"/cards/"
					{ @func private_string }
					".conf"
				]
			}
		]
		errors false
	}
]

#
# defaults
#

# show all name hints also for definitions without hint {} section
defaults.namehint.showall off
# show just basic name hints
defaults.namehint.basic on
# show extended name hints
defaults.namehint.extended off
#
defaults.ctl.card 0
defaults.pcm.card 0
defaults.pcm.device 0
defaults.pcm.subdevice -1
defaults.pcm.nonblock 1
defaults.pcm.ipc_key 5678293
defaults.pcm.ipc_gid audio
defaults.pcm.ipc_perm 0660
defaults.pcm.dmix.max_periods 0
defaults.pcm.dmix.rate 48000
defaults.pcm.dmix.format "unchanged"
defaults.pcm.dmix.card defaults.pcm.card
defaults.pcm.dmix.device defaults.pcm.device
defaults.pcm.dsnoop.card defaults.pcm.card
defaults.pcm.dsnoop.device defaults.pcm.device
defaults.pcm.front.card defaults.pcm.card
defaults.pcm.front.device defaults.pcm.device
defaults.pcm.rear.card defaults.pcm.card
defaults.pcm.rear.device defaults.pcm.device
defaults.pcm.center_lfe.card defaults.pcm.card
defaults.pcm.center_lfe.device defaults.pcm.device
defaults.pcm.side.card defaults.pcm.card
defaults.pcm.side.device defaults.pcm.device
defaults.pcm.surround40.card defaults.pcm.card
defaults.pcm.surround40.device defaults.pcm.device
defaults.pcm.surround41.card defaults.pcm.card
defaults.pcm.surround41.device defaults.pcm.device
defaults.pcm.surround50.card defaults.pcm.card
defaults.pcm.surround50.device defaults.pcm.device
defaults.pcm.surround51.card defaults.pcm.card
defaults.pcm.surround51.device defaults.pcm.device
defaults.pcm.surround71.card defaults.pcm.card
defaults.pcm.surround71.device defaults.pcm.device
defaults.pcm.iec958.card defaults.pcm.card
defaults.pcm.iec958.device defaults.pcm.device
defaults.pcm.modem.card defaults.pcm.card
defaults.pcm.modem.device defaults.pcm.device
# truncate files via file or tee PCM
defaults.pcm.file_format	"raw"
defaults.pcm.file_truncate	true
defaults.rawmidi.card 0
defaults.rawmidi.device 0
defaults.rawmidi.subdevice -1
defaults.hwdep.card 0
defaults.hwdep.device 0
defaults.timer.class 2
defaults.timer.sclass 0
defaults.timer.card 0
defaults.timer.device 0
defaults.timer.subdevice 0

#
#  PCM interface
#

# redirect to load-on-demand extended pcm definitions
pcm.cards cards.pcm

pcm.default cards.pcm.default
pcm.front cards.pcm.front
pcm.rear cards.pcm.rear
pcm.center_lfe cards.pcm.center_lfe
pcm.side cards.pcm.side
pcm.surround40 cards.pcm.surround40
pcm.surround41 cards.pcm.surround41
pcm.surround50 cards.pcm.surround50
pcm.surround51 cards.pcm.surround51
pcm.surround71 cards.pcm.surround71
pcm.iec958 cards.pcm.iec958
pcm.spdif iec958
pcm.hdmi cards.pcm.hdmi
pcm.dmix cards.pcm.dmix
pcm.dsnoop cards.pcm.dsnoop
pcm.modem cards.pcm.modem
pcm.phoneline cards.pcm.phoneline

pcm.hw {
	@args [ CARD DEV SUBDEV ]
	@args.CARD {
		type string
		default {
			@func getenv
			vars [
				ALSA_PCM_CARD
				ALSA_CARD
			]
			default {
				@func refer
				name defaults.pcm.card
			}
		}
	}
	@args.DEV {
		type integer
		default {
			@func igetenv
			vars [
				ALSA_PCM_DEVICE
			]
			default {
				@func refer
				name defaults.pcm.device
			}
		}
	}
	@args.SUBDEV {
		type integer
		default {
			@func refer
			name defaults.pcm.subdevice
		}
	}		
	type hw
	card $CARD
	device $DEV
	subdevice $SUBDEV
	hint {
		show {
			@func refer
			name defaults.namehint.extended
		}
		description "Direct hardware device without any conversions"
	}
}

pcm.plughw {
	@args [ CARD DEV SUBDEV ]
	@args.CARD {
		type string
		default {
			@func getenv
			vars [
				ALSA_PCM_CARD
				ALSA_CARD
			]
			default {
				@func refer
				name defaults.pcm.card
			}
		}
	}
	@args.DEV {
		type integer
		default {
			@func igetenv
			vars [
				ALSA_PCM_DEVICE
			]
			default {
				@func refer
				name defaults.pcm.device
			}
		}
	}
	@args.SUBDEV {
		type integer
		default {
			@func refer
			name defaults.pcm.subdevice
		}
	}		
	type plug
	slave.pcm {
		type hw
		card $CARD
		device $DEV
		subdevice $SUBDEV
	}
	hint {
		show {
			@func refer
			name defaults.namehint.extended
		}
		description "Hardware device with all software conversions"
	}
}

pcm.plug {
	@args [ SLAVE ]
	@args.SLAVE {
		type string
	}
	type plug
	slave.pcm $SLAVE
}

pcm.shm {
	@args [ SOCKET PCM ]
	@args.SOCKET {
		type string
	}
	@args.PCM {
		type string
	}
	type shm
	server $SOCKET
	pcm $PCM
}

pcm.tee {
	@args [ SLAVE FILE FORMAT ]
	@args.SLAVE {
		type string
	}
	@args.FILE {
		type string
	}
	@args.FORMAT {
		type string
		default {
			@func refer
			name defaults.pcm.file_format
		}
	}
	type file
	slave.pcm $SLAVE
	file $FILE
	format $FORMAT
	truncate {
		@func refer
		name defaults.pcm.file_truncate
	}
}

pcm.file {
	@args [ FILE FORMAT ]
	@args.FILE {
		type string
	}
	@args.FORMAT {
		type string
		default {
			@func refer
			name defaults.pcm.file_format
		}
	}
	type file
	slave.pcm null
	file $FILE
	format $FORMAT
	truncate {
		@func refer
		name defaults.pcm.file_truncate
	}
}

pcm.null {
	type null
	hint {
		show {
			@func refer
			name defaults.namehint.basic
		}
		description "Discard all samples (playback) or generate zero samples (capture)"
	}
}

#
#  Control interface
#
	
ctl.default {
	type hw
	card {
		@func getenv
		vars [
			ALSA_CTL_CARD
			ALSA_CARD
		]
		default {
			@func refer
			name defaults.ctl.card
		}
	}
}

ctl.hw {
	@args [ CARD ]
	@args.CARD {
		type string
		default {
			@func getenv
			vars [
				ALSA_CTL_CARD
				ALSA_CARD
			]
			default {
				@func refer
				name defaults.ctl.card
			}
		}
	}
	type hw
	card $CARD
}

ctl.shm {
	@args [ SOCKET CTL ]
	@args.SOCKET {
		type string
	}
	@args.CTL {
		type string
	}
	type shm
	server $SOCKET
	ctl $CTL
}

#
#  RawMidi interface
#

rawmidi.default {
	type hw
	card {
		@func getenv
		vars [
			ALSA_RAWMIDI_CARD
			ALSA_CARD
		]
		default {
			@func refer
			name defaults.rawmidi.card
		}
	}
	device {
		@func igetenv
		vars [
			ALSA_RAWMIDI_DEVICE
		]
		default {
			@func refer
			name defaults.rawmidi.device
		}
	}
}

rawmidi.hw {
	@args [ CARD DEV SUBDEV ]
	@args.CARD {
		type string
		default {
			@func getenv
			vars [
				ALSA_RAWMIDI_CARD
				ALSA_CARD
			]
			default {
				@func refer
				name defaults.rawmidi.card
			}
		}
	}
	@args.DEV {
		type integer
		default {
			@func igetenv
			vars [
				ALSA_RAWMIDI_DEVICE
			]
			default {
				@func refer
				name defaults.rawmidi.device
			}
		}
	}
	@args.SUBDEV {
		type integer
		default -1
	}
	type hw
	card $CARD
	device $DEV
	subdevice $SUBDEV
	hint {
		description "Direct rawmidi driver device"
		device $DEV
	}
}

rawmidi.virtual {
	@args [ MERGE ]
	@args.MERGE {
		type string
		default 1
	}
	type virtual
	merge $MERGE
}

#
#  Sequencer interface
#

seq.default {
	type hw
}

seq.hw {
	type hw
}

#
#  HwDep interface
#

hwdep.default {
	type hw
	card {
		@func getenv
		vars [
			ALSA_HWDEP_CARD
			ALSA_CARD
		]
		default {
			@func refer
			name defaults.hwdep.card
		}
	}
	device {
		@func igetenv
		vars [
			ALSA_HWDEP_DEVICE
		]
		default {
			@func refer
			name defaults.hwdep.device
		}
	}
}

hwdep.hw {
	@args [ CARD DEV ]
	@args.CARD {
		type string
		default {
			@func getenv
			vars [
				ALSA_HWDEP_CARD
				ALSA_CARD
			]
			default {
				@func refer
				name defaults.hwdep.card
			}
		}
	}
	@args.DEV {
		type integer
		default {
			@func igetenv
			vars [
				ALSA_HWDEP_DEVICE
			]
			default {
				@func refer
				name defaults.hwdep.device
			}
		}
	}
	type hw
	card $CARD
	device $DEV
}

#
#  Timer interface
#

timer_query.default {
	type hw
}

timer_query.hw {
	type hw
}

timer.default {
	type hw
	class {
		@func refer
		name defaults.timer.class
	}
	sclass {
		@func refer
		name defaults.timer.sclass
	}
	card {
		@func refer
		name defaults.timer.card
	}
	device {
		@func refer
		name defaults.timer.device
	}
	subdevice {
		@func refer
		name defaults.timer.subdevice
	}
	hint.description "Default direct hardware timer device"
}

timer.hw {
	@args [ CLASS SCLASS CARD DEV SUBDEV ]
	@args.CLASS {
		type integer
		default {
			@func refer
			name defaults.timer.class
		}
	}
	@args.SCLASS {
		type integer
		default {
			@func refer
			name defaults.timer.sclass
		}
	}
	@args.CARD {
		type string
		default {
			@func refer
			name defaults.timer.card
		}
	}
	@args.DEV {
		type integer
		default {
			@func refer
			name defaults.timer.device
		}
	}
	@args.SUBDEV {
		type integer
		default {
			@func refer
			name defaults.timer.subdevice
		}
	}
	type hw
	class $CLASS
	sclass $SCLASS
	card $CARD
	device $DEV
	subdevice $SUBDEV
}

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] alsa-lib: snd_device_name_hint misbehaving

[Takashi Iwai]

At Mon, 02 Nov 2009 12:58:20 -0500,
John Lindgren wrote:
> 
> On Mon, 2009-11-02 at 15:55 +0100, Takashi Iwai wrote:
> > Thanks.  I guess this depends on the config files.
> > Could you attach your ones?
> 
> I can reproduce with only the stock /usr/share/alsa/alsa.conf from
> Debian installed if I try to use the "null" device after
> snd_device_name_hint.  /usr/share/alsa/alsa.conf is attached.

Thanks!  I could reproduce the problem on my machine, too.

Now I think I found the culprit.  This is not in the core conf.c code,
but it's in namehint.c.  Try the patch below.

The point is that the variable "res" can be two different instances
in try_config().  One is the result of snd_config_search_definition().
In this case, res is the copied (expanded) objects.

This one is freed in the middle of try_config(), and then res is
assigned again to another one by snd_config_search_alias_hooks().
This guy is no expanded object but a reference.

However, the original code calls snd_config_free(res) no matter which
object is.  So, freeing the latter one results in the breakage of the
config tree.

The patch adds the check of the object type to be freed or not.


Takashi

---
diff --git a/src/control/namehint.c b/src/control/namehint.c
index e878f83..a134ed7 100644
--- a/src/control/namehint.c
+++ b/src/control/namehint.c
@@ -219,6 +219,7 @@ static int try_config(struct hint_list *list,
 	const char *str;
 	int err = 0, level;
 	long dev = list->device;
+	int cleanup_res = 0;
 
 	list->device_input = -1;
 	list->device_output = -1;
@@ -244,6 +245,7 @@ static int try_config(struct hint_list *list,
 	snd_lib_error_set_handler(eh);
 	if (err < 0)
 		goto __skip_add;
+	cleanup_res = 1;
 	err = -EINVAL;
 	if (snd_config_get_type(res) != SND_CONFIG_TYPE_COMPOUND)
 		goto __cleanup;
@@ -330,6 +332,7 @@ static int try_config(struct hint_list *list,
 	    	goto __hint;
 	snd_config_delete(res);
 	res = NULL;
+	cleanup_res = 0;
 	if (strchr(buf, ':') != NULL)
 		goto __ok;
 	/* find, if all parameters have a default, */
@@ -379,7 +382,7 @@ static int try_config(struct hint_list *list,
 	      	err = hint_list_add(list, buf, buf1);
 	}
       __skip_add:
-      	if (res)
+	if (res && cleanup_res)
 	      	snd_config_delete(res);
 	if (buf1)
 		free(buf1);

Re: [PATCH] alsa-lib: snd_device_name_hint misbehaving

[Takashi Iwai]

At Tue, 03 Nov 2009 08:09:34 +0100,
I wrote:
> 
> At Mon, 02 Nov 2009 12:58:20 -0500,
> John Lindgren wrote:
> > 
> > On Mon, 2009-11-02 at 15:55 +0100, Takashi Iwai wrote:
> > > Thanks.  I guess this depends on the config files.
> > > Could you attach your ones?
> > 
> > I can reproduce with only the stock /usr/share/alsa/alsa.conf from
> > Debian installed if I try to use the "null" device after
> > snd_device_name_hint.  /usr/share/alsa/alsa.conf is attached.
> 
> Thanks!  I could reproduce the problem on my machine, too.
> 
> Now I think I found the culprit.  This is not in the core conf.c code,
> but it's in namehint.c.  Try the patch below.
> 
> The point is that the variable "res" can be two different instances
> in try_config().  One is the result of snd_config_search_definition().
> In this case, res is the copied (expanded) objects.
> 
> This one is freed in the middle of try_config(), and then res is
> assigned again to another one by snd_config_search_alias_hooks().
> This guy is no expanded object but a reference.
> 
> However, the original code calls snd_config_free(res) no matter which
> object is.  So, freeing the latter one results in the breakage of the
> config tree.
> 
> The patch adds the check of the object type to be freed or not.

Since no problem is found with valgrind, I applied the patch to git tree
now.


thanks,

Takashi

Re: [PATCH] alsa-lib: snd_device_name_hint misbehaving

[John Lindgren]

On Tue, 2009-11-03 at 09:03 +0100, Takashi Iwai wrote:
> Since no problem is found with valgrind, I applied the patch to git tree
> now.

Thanks.  I compiled the git version and the problem is fixed. Here are
the other changes, split into four separate patches:

* Card-independent devices such as "null" or "pulse" should only be
added once, not once for each card.

>>>>>>

diff --git a/src/control/namehint.c b/src/control/namehint.c
index a134ed7..408b36f 100644
--- a/src/control/namehint.c
+++ b/src/control/namehint.c
@@ -456,10 +456,6 @@ static int add_card(struct hint_list *list, int card)
 			list->device = -1;
 			err = try_config(list, list->siface, str);
 		}
-		if (err < 0) {
-			list->card = -1;
-			err = try_config(list, list->siface, str);
-		}
 		if (err == -ENOMEM)
 			goto __error;
 	}
@@ -485,6 +481,29 @@ static int get_card_name(struct hint_list *list, int card)
 	return 0;
 }
 
+static int add_software_devices(struct hint_list *list)
+{
+	int err;
+	snd_config_t *conf, *n;
+	snd_config_iterator_t i, next;
+	const char *str;
+
+	err = snd_config_search(snd_config, list->siface, &conf);
+	if (err < 0)
+		return err;
+	snd_config_for_each(i, next, conf) {
+		n = snd_config_iterator_entry(i);
+		if (snd_config_get_id(n, &str) < 0)
+			continue;
+		list->card = -1;
+		list->device = -1;
+		err = try_config(list, list->siface, str);
+		if (err == -ENOMEM)
+			return -ENOMEM;
+	}
+	return 0;
+}
+
 /**
  * \brief Return string list with device name hints.
  * \param card Card number or -1 (means all cards)
@@ -546,6 +565,7 @@ int snd_device_name_hint(int card, const char *iface, void ***hints)
 		if (err >= 0)
 			err = add_card(&list, card);
 	} else {
+		add_software_devices(&list);
 		err = snd_card_next(&card);
 		if (err < 0)
 			goto __error;

<<<<<<

* Allow snd_device_name_hint to search for mixer devices.

>>>>>>

diff --git a/src/control/namehint.c b/src/control/namehint.c
index 408b36f..34338dc 100644
--- a/src/control/namehint.c
+++ b/src/control/namehint.c
@@ -554,6 +554,8 @@ int snd_device_name_hint(int card, const char *iface, void ***hints)
 		list.iface = SND_CTL_ELEM_IFACE_SEQUENCER;
 	else if (strcmp(iface, "hwdep") == 0)
 		list.iface = SND_CTL_ELEM_IFACE_HWDEP;
+	else if (strcmp(iface, "ctl") == 0)
+		list.iface = SND_CTL_ELEM_IFACE_MIXER;
 	else
 		return -EINVAL;
 	list.show_all = 0;

<<<<<<

* list->card is wrongly assumed to be initialized, but the previous
initialization is within a conditional that is false when only
card-independent devices are found.  (This is the case when searching
for mixers on my system; the end result is that the "pulse" mixer is
listed three times.)

>>>>>>

diff --git a/src/control/namehint.c b/src/control/namehint.c
index 34338dc..78572d8 100644
--- a/src/control/namehint.c
+++ b/src/control/namehint.c
@@ -453,6 +453,7 @@ static int add_card(struct hint_list *list, int card)
 		if (err == -EXDEV)
 			continue;
 		if (err < 0) {
+			list->card = card;
 			list->device = -1;
 			err = try_config(list, list->siface, str);
 		}

<<<<<<

* Speed up repeated calls to snd_dlopen by caching the path to
libasound.so; this reduces the instructions executed by
snd_device_name_hint by 40 percent.

>>>>>>

diff --git a/src/dlmisc.c b/src/dlmisc.c
index c882cdc..3cca0f0 100644
--- a/src/dlmisc.c
+++ b/src/dlmisc.c
@@ -54,9 +54,13 @@ void *snd_dlopen(const char *name, int mode)
 #else
 #ifdef HAVE_LIBDL
 	if (name == NULL) {
-		Dl_info dlinfo;
-		if (dladdr(snd_dlopen, &dlinfo) > 0)
-			name = dlinfo.dli_fname;
+		static const char * self = NULL;
+		if (self == NULL) {
+			Dl_info dlinfo;
+			if (dladdr(snd_dlopen, &dlinfo) > 0)
+				self = dlinfo.dli_fname;
+		}
+		name = self;
 	}
 #endif
 #endif

<<<<<<

Peace,
John Lindgren

Re: [PATCH] alsa-lib: snd_device_name_hint misbehaving

[Jaroslav Kysela]

On Tue, 3 Nov 2009, John Lindgren wrote:

> On Tue, 2009-11-03 at 09:03 +0100, Takashi Iwai wrote:
>> Since no problem is found with valgrind, I applied the patch to git tree
>> now.
>
> Thanks.  I compiled the git version and the problem is fixed. Here are
> the other changes, split into four separate patches:

I applied your changes. But please ... send one patch in one e-mail in 
future to reduce our work with patch importing. Also, it makes sense to 
have one-line subject for patch and Signed-off-by: line in patches.

 				Thanks,
 					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: [PATCH] alsa-lib: snd_device_name_hint misbehaving

[John Lindgren]

[-- Attachment #1: Type: text/plain, Size: 441 bytes --]

Raymond Yau wrote:
> Is there any test program for this change since currently only "hw" and
> "pulse" can be listed by name hint ?
> 
> * Add "ctl" to the list of device types that snd_device_name_hint can
> search for.

The development snapshot of Audacious is the only program that uses
snd_device_name_hint to list mixer devices, to my knowledge.  Attached
are screenshots of the selector menu before and after the patch.

John Lindgren

[-- Attachment #2: after.png --]
[-- Type: image/png, Size: 12232 bytes --]

[-- Attachment #3: before.png --]
[-- Type: image/png, Size: 9319 bytes --]

[-- Attachment #4: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel