Re: [PATCH] [updated] pcm: rate: Add capability to pass configuration node to plugins

[Takashi Iwai <tiwai@suse.de>]

On Fri, 17 Feb 2017 18:44:47 +0100,
Alan Young wrote:
> 
> On 17/02/17 16:53, Takashi Iwai wrote:
> > The problem is that you pass always the compound object as the
> > converter argument.  As you already suggested, there are two cases:
> > one is a string array and another is a compound with optional args.
> > In your code, the first iteration doesn't check which type is.  So it
> > always fails if the string array is passed.
> 
> Well, it does actually work in both cases but I guess that the plugin
> could get passed an unexpected type of compound config converter
> parameter in some cases.
> 
> >
> > That is, the right implementation is to check whether the given
> > compound is a string array is.  If yes, it goes to the old style
> > loop (and you can check "name" argument properly).  If not, it goes
> > with the new compound argument.  That's simple enough, and more
> > understandable the condition you used for the loop termination.
> The following makes the difference more explicit What do you think?
> 
>     else if (snd_config_get_type(converter) == SND_CONFIG_TYPE_COMPOUND) {
>         snd_config_iterator_t i, next;
>         int pos = 0, is_array = 0;
>         /*
>          * If the convertor compound is an array of alternatives then
> the id of
>          * the first element will be "0" (or maybe NULL). Otherwise
> assume it is
>          * a structure and must have a "name" id to identify the
> converter type.
>          */
>         snd_config_for_each(i, next, converter) {
>             snd_config_t *n = snd_config_iterator_entry(i);
>             const char *id;
>             snd_config_get_id(n, &id);
>             if (pos++ == 0 && (!id || strcmp(id, "0") == 0))
>                 is_array = 1;
>             if (!is_array && strcmp(id, "name") != 0)
>                 continue;
>             if (snd_config_get_string(n, &type) < 0)
>                 break;
>             err = rate_open_func(rate, type, is_array ? NULL :
> converter, 0);
>             if (!err || !is_array)
>                 break;
>         }

It becomes too complex by mixing up things in a single loop.
Let's take it just simple like: 

	if (is_string_array(converter)) {
		snd_config_for_each(i, next, converter) {
			// like the old way
		}
	} else {
		// handle for the new compound type
	}


Takashi