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

Tanu Kaskinen tanuk at iki.fi
Tue Oct 2 14:03:57 PDT 2012


On Mon, 2012-10-01 at 15:48 +0200, David Henningsson wrote:
> 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.

Ok, thanks for clarifying that. Since it was unclear what we decided,
effectively we didn't decide anything. Therefore, until we decide
something else, I think the policy stays as it has been. So, this will
cause complaints:

if (a) {
    foo();
}

and anything more complicated is unregulated (i.e. no complaints, braces
or not).

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

I think the assertions on NULL pointers given to pa_hashmap_* have
value, so I'm opposed to that proposal. I don't see any significant
problems with always initializing the containers, do you? The increased
memory use should be negligible.

-- 
Tanu



More information about the pulseaudio-discuss mailing list