[alsa-devel] [PATCH 1/8] ALSA: pcm: add a helper function to constrain mask-type parameters

[Takashi Sakamoto]

Hi,

On Jun 8 2017 16:35, Takashi Iwai wrote:
> On Thu, 08 Jun 2017 09:28:37 +0200,
> Takashi Sakamoto wrote:
>>
>> Hi,
>>
>> On Jun 8 2017 16:10, Takashi Iwai wrote:
>>> On Thu, 08 Jun 2017 01:10:19 +0200,
>>> Takashi Sakamoto wrote:
>>>>
>>>> Application of constraints to mask-type parameters for PCM substream is
>>>> done in a call of snd_pcm_hw_refine(), while the function includes much
>>>> codes and is not enough friendly to readers.
>>>>
>>>> This commit splits the codes to a separated function so that readers can
>>>> get it easily. I leave desicion into compilers to merge the function into
>>>> its callee.
>>>>
>>>> Signed-off-by: Takashi Sakamoto <o-takashi at sakamocchi.jp>
>>>> ---
>>>>    sound/core/pcm_native.c | 57 ++++++++++++++++++++++++++++++++-----------------
>>>>    1 file changed, 38 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
>>>> index 2bde07a4a87f..b3e8aab3915e 100644
>>>> --- a/sound/core/pcm_native.c
>>>> +++ b/sound/core/pcm_native.c
>>>> ...
>>>> +	struct snd_mask __maybe_unused old_mask;
>>>
>>> Do we really need __maybe_unused?  Drop it as much as possible.
>>> IMO, it can be uglier than ifdef, since you don't know why it's
>>> unused.  With ifdef, at least, you have an idea about the condition.
>>
>> In kernel documentation[1], we can see below suggestion.
>>
>> "20) Conditional Compilation
>> ...
>> If you have a function or variable which may potentially go unused in a
>> particular configuration, and the compiler would warn about its
>> definition going unused, mark the definition as __maybe_unused rather
>> than wrapping it in a preprocessor conditional.  (However, if a
>> function or variable *always* goes unused, delete it.)"
>>
>> I'll follow this.
> 
> It doesn't answer my question.  Without __maybe_unused in your code,
> do you get the compile warning?  If yes, why?

Oh, I cannot see any warnings when disabling tracepoints or SND_DEBUG, 
as you said... I surely have a bias from my experiences for 
firewire-motu driver...

> Well, I see the usage already in your patch for the tracepoints.
> But __maybe_unused is really ugly, and should be avoided as much as
> possible.
> 
> The text above doesn't recommend to use it blindly.   It's the last
> resort.

OK. We can avoid using __maybe_unused for function local variable in 
this case. I'll drop the keyword in next v2 patchset.


Thank you

Takashi Sakamoto