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()? -- Tanu