From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH 1/8] ALSA: pcm: add a helper function to constrain mask-type parameters Date: Thu, 08 Jun 2017 09:35:24 +0200 Message-ID: References: <20170607231026.23383-1-o-takashi@sakamocchi.jp> <20170607231026.23383-2-o-takashi@sakamocchi.jp> <41daafb7-8185-9e3e-147b-22710c3ef885@sakamocchi.jp> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 5DD142671CA for ; Thu, 8 Jun 2017 09:35:25 +0200 (CEST) In-Reply-To: <41daafb7-8185-9e3e-147b-22710c3ef885@sakamocchi.jp> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Takashi Sakamoto Cc: alsa-devel@alsa-project.org, kuninori.morimoto.gx@renesas.com List-Id: alsa-devel@alsa-project.org 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 > >> --- > >> 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? 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. thanks, Takashi