Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751830AbbGODBu (ORCPT ); Tue, 14 Jul 2015 23:01:50 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:49018 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751189AbbGODBt (ORCPT ); Tue, 14 Jul 2015 23:01:49 -0400 Date: Tue, 14 Jul 2015 20:01:49 -0700 From: Greg Kroah-Hartman To: Luis de Bethencourt Cc: linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, Kirk Reiser , speakup@linux-speakup.org, Domagoj Trsan , Samuel Thibault , Dan Carpenter , Chris Brannon Subject: Re: [PATCH 2/2] staging: speakup: else is not useful after a return Message-ID: <20150715030149.GA14686@kroah.com> References: <20150625125650.GA20230@goodgumbo.baconseed.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150625125650.GA20230@goodgumbo.baconseed.org> User-Agent: Mutt/1.5.23+102 (2ca89bed6448) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2394 Lines: 67 On Thu, Jun 25, 2015 at 02:56:52PM +0200, Luis de Bethencourt wrote: > Correct a checkpatch.pl warning regarding > WARNING: else is not generally useful after a break or return > drivers/staging/speakup/keyhelp.c:185: > > Changing the order of the if blocks, but not the logic, to avoid this > warning. The block after else will run if the blocks KT_CUR or KT_LATIN > set cur_item instead of returning. Don't do things just because checkpatch tells you, especially when it doesn't make much sense to do so. For example, this patch :) > > Signed-off-by: Luis de Bethencourt > --- > drivers/staging/speakup/keyhelp.c | 32 ++++++++++++++++---------------- > 1 file changed, 16 insertions(+), 16 deletions(-) > > diff --git a/drivers/staging/speakup/keyhelp.c b/drivers/staging/speakup/keyhelp.c > index 02d5c70..8133b6e 100644 > --- a/drivers/staging/speakup/keyhelp.c > +++ b/drivers/staging/speakup/keyhelp.c > @@ -151,22 +151,7 @@ int spk_handle_help(struct vc_data *vc, u_char type, u_char ch, u_short key) > > if (letter_offsets[0] == -1) > help_init(); > - if (type == KT_LATIN) { > - if (ch == SPACE) { > - spk_special_handler = NULL; > - synth_printf("%s\n", spk_msg_get(MSG_LEAVING_HELP)); > - return 1; > - } > - ch |= 32; /* lower case */ > - if (ch < 'a' || ch > 'z') > - return -1; > - if (letter_offsets[ch-'a'] == -1) { > - synth_printf(spk_msg_get(MSG_NO_COMMAND), ch); > - synth_printf("\n"); > - return 1; > - } > - cur_item = letter_offsets[ch-'a']; > - } else if (type == KT_CUR) { > + if (type == KT_CUR) { > if (ch == 0 > && (MSG_FUNCNAMES_START + cur_item + 1) <= > MSG_FUNCNAMES_END) > @@ -182,6 +167,21 @@ int spk_handle_help(struct vc_data *vc, u_char type, u_char ch, u_short key) > synth_printf("%s\n", spk_msg_get(MSG_HELP_INFO)); > build_key_data(); /* rebuild each time in case new mapping */ > return 1; > + } else if (type == KT_LATIN) { One thing, this else isn't needed. Or switch this to a case statement instead? That might make it more obvious as to what is going on, try that and see if it looks better. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/