From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754297AbcHVE4Y (ORCPT ); Mon, 22 Aug 2016 00:56:24 -0400 Received: from mga01.intel.com ([192.55.52.88]:47119 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752196AbcHVE4W (ORCPT ); Mon, 22 Aug 2016 00:56:22 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,558,1464678000"; d="scan'208";a="1045168146" Date: Mon, 22 Aug 2016 10:31:56 +0530 From: Vinod Koul To: Julia Lawall Cc: SF Markus Elfring , alsa-devel@alsa-project.org, Jaroslav Kysela , Takashi Iwai , LKML , kernel-janitors@vger.kernel.org Subject: Re: [PATCH 2/2] ALSA: compress: Reduce the scope for two variables in snd_compr_set_params() Message-ID: <20160822050156.GD2890@localhost> References: <566ABCD9.1060404@users.sourceforge.net> <7233eb00-a941-4935-dce2-7cb907272d41@users.sourceforge.net> <03eac1fe-ff33-1759-9c2a-7914f1c6de45@users.sourceforge.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Aug 21, 2016 at 04:36:22PM -0400, Julia Lawall wrote: > > > On Sun, 21 Aug 2016, SF Markus Elfring wrote: > > > From: Markus Elfring > > Date: Sun, 21 Aug 2016 21:26:18 +0200 > > > > Reduce the scope for the local variables to an if branch. > > > > Signed-off-by: Markus Elfring > > --- > > sound/core/compress_offload.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c > > index 583d407..b43aec5 100644 > > --- a/sound/core/compress_offload.c > > +++ b/sound/core/compress_offload.c > > @@ -545,14 +545,14 @@ static int snd_compress_check_input(struct snd_compr_params *params) > > static int > > snd_compr_set_params(struct snd_compr_stream *stream, unsigned long arg) > > { > > - struct snd_compr_params *params; > > - int retval; > > - > > if (stream->runtime->state == SNDRV_PCM_STATE_OPEN) { > > /* > > * we should allow parameter change only when stream has been > > * opened not in other cases > > */ > > + int retval; > > + struct snd_compr_params *params; > > I don't like this at all. Local variables should be at the top of the > function, not hiding under 4 lines of comments in the middle of the code. I agree with you this, it doesn't help IMO as well -- ~Vinod