[pulseaudio-discuss] [RFC PATCH] alsa-mixer: Cache failure to open inputs

[David Henningsson]

On 09/25/2012 06:46 PM, Tanu Kaskinen wrote:
> On Mon, 2012-09-24 at 17:02 +0200, David Henningsson wrote:
>> On 09/24/2012 03:33 PM, Tanu Kaskinen wrote:
>>> Ok, I don't have a problem with this example. But the code in your patch
>>> has different structure: instead of a simple match_found() call, the
>>> inner code contains a three-line block.
>>
>> Well, I still think
>>
>> for (i)
>>       for (j)
>>           if (a[i] == b[j]) {
>>               three();
>>               line();
>>               block();
>>           }
>>
>> ...looks better than
>>
>> for (i)
>>       for (j)
>>           if (a[i] == b[j]) {
>>               three();
>>               line();
>>               block();
>>           }
>>       }
>> }
>>
>> ...but maybe that's just me.
>
> I certainly prefer the latter, though this is not a very important thing
> for me. But I'm now left wondering what you thought we agreed earlier -
> unfortunately I don't have the logs, but I think we agreed to disallow
> some cases of omitting the braces. If you think that the first example
> would still be allowed, I don't know what case would be left to
> disallow.

I don't recall for sure, but I think I was imagining multiline 
statements, e g

if (i)
    this_is_a_very_long_statement(with_multiple, parameters,
                                  that_maybe_are_functions(themselves));

vs

if (i) {
    this_is_a_very_long_statement(with_multiple, parameters,
                                  that_maybe_are_functions(themselves));
}

in which case the latter is okay. Anyway, if it's not important, it's 
just another trap to fall in when you try to fix somebody's real problem.

>>> Fair enough. I don't like the check, because it implies that it's
>>> possible that output_mappings is empty, which is not true, but I see how
>>> this can prevent a bug in the future (not a severe bug, because only a
>>> minor optimization would be skipped, but a bug anyway). I think it would
>>> be best to always create the output_mappings idxset so that the code
>>> doesn't have to worry about it being NULL. If you agree, I'll file a
>>> bug, since the issue is separate from this patch and I don't plan to
>>> write the patch right away, nor ask you to do so.
>>
>> I don't know if we have a generic opinion on whether we prefer NULL
>> idxsets or empty idxsets? Or determine on a case-by-case basis?
>
> I don't think there has been any rule so far, or if there has been, it
> has been to favor NULL. I would definitely choose to always initialize
> idxsets and hashmaps if there's no particular reason to distinguish
> between NULL and an empty container. (I have even sent some patches that
> ensure that certain idxsets or hashmaps are always non-NULL, but I don't
> remember if they have been reviewed yet.) It makes the code simpler when
> you don't have to worry about the container being NULL.

Another option could be to make pa_hashmap_* to treat NULL pointers as 
empty containers. I don't know if that's better though.


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