[pulseaudio-discuss] [PATCH] Alsa: refactor pa_alsa_profile_set_probe

[David Henningsson]

On 12/31/2011 03:48 PM, Tanu Kaskinen wrote:
> Hi,
>
> Here's finally a review for this patch. Sorry for the delay.
>
> On Fri, 2011-12-02 at 18:14 +0100, David Henningsson wrote:
>> +/* Closes pcm handles in "last" that are not needed for probing "keep". */
>> +static void profile_close_pcm(pa_alsa_profile *last, pa_alsa_profile *keep) {
>> +    pa_alsa_mapping *m;
>> +    uint32_t idx;
>> +
>> +    if (!last)
>> +        return;
>> +
>> +    /* Close PCMs from the last iteration we don't need anymore */
>> +    if (last->output_mappings)
>> +        PA_IDXSET_FOREACH(m, last->output_mappings, idx) {
>> +
>> +            if (!m->output_pcm)
>> +                break;
>
> This looks strange. Should this be continue instead of break?
>
>> +            if (last->supported)
>> +                m->supported++;
>
> I'm not sure that this is the best place to mark the mapping as
> supported, because this function is supposed to be about closing the
> profile pcm handles. OTOH, I guess doing it here is the most convenient
> place (doing it somewhere else would probably require an extra loop).
> Maybe rename the function to profile_finalize_probing()?
>

I think both suggestions make sense. The break vs continue is 
copy-pasted from the old code, and it is very unusual to have more than 
one mapping per direction in a profile (right?) so I guess that's why it 
hasn't been tested much.

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic