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

[Tanu Kaskinen]

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.

> > 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.

-- 
Tanu