From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH] [updated] pcm: rate: Add capability to pass configuration node to plugins Date: Fri, 17 Feb 2017 18:59:40 +0100 Message-ID: References: <6d7c74b7-da90-5ae9-c355-c6884b2edf09@gmail.com> <7b3c655f-4b7a-e10a-90ff-5cb480332f3f@IEE.org> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 308132671DB for ; Fri, 17 Feb 2017 18:59:41 +0100 (CET) In-Reply-To: <7b3c655f-4b7a-e10a-90ff-5cb480332f3f@IEE.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Alan Young Cc: alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org 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