From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933229AbdC3HwQ (ORCPT ); Thu, 30 Mar 2017 03:52:16 -0400 Received: from mail-wr0-f178.google.com ([209.85.128.178]:35537 "EHLO mail-wr0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932650AbdC3HwO (ORCPT ); Thu, 30 Mar 2017 03:52:14 -0400 Date: Thu, 30 Mar 2017 09:52:10 +0200 From: Gioh Kim To: NeilBrown Cc: jes.sorensen@gmail.com, linux-raid@vger.kernel.org, linux-kernel@vger.kernel.org, Wol Subject: Re: [PATCH 2/2] mdadm.c: fix compile error "switch condition has boolean value" Message-ID: <20170330075209.GF17378@ws00837> References: <1490780434-8720-1-git-send-email-gi-oh.kim@profitbricks.com> <1490780434-8720-2-git-send-email-gi-oh.kim@profitbricks.com> <87lgrnu5k4.fsf@notabene.neil.brown.name> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87lgrnu5k4.fsf@notabene.neil.brown.name> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 30, 2017 at 08:38:35AM +1100, NeilBrown wrote: > On Thu, Mar 30 2017, jes.sorensen@gmail.com wrote: > > > Gioh Kim writes: > >> Remove a boolean expression in switch condition > >> to prevent compile error of some compilers. > > > > Please be specific, which compile is unable to handle this? > > > >> Signed-off-by: Gioh Kim > >> --- > >> mdadm.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/mdadm.c b/mdadm.c > >> index 08ddcab..a98a051 100644 > >> --- a/mdadm.c > >> +++ b/mdadm.c > >> @@ -1905,11 +1905,11 @@ static int misc_list(struct mddev_dev *devlist, > >> rv |= SetAction(dv->devname, c->action); > >> continue; > >> } > >> - switch(dv->devname[0] == '/') { > >> - case 0: > >> + switch(dv->devname[0]) { > >> + default: > >> mdfd = open_dev(dv->devname); > >> if (mdfd >= 0) break; > >> - case 1: > >> + case '/': > >> mdfd = open_mddev(dv->devname, 1); > >> } > >> if (mdfd>=0) { > > > > While I agree the original code is ugly, I am not convinced your > > replacement is a lot prettier. > > > > Maybe > > if (dv->devname[0] == '/' || > (mdfd = open_dev(dv->devname)) < 0) > mdfd = open_mddev(dv->devname, 1); > > ?? > NeilBrown Yes, the initial version I thought was: if (dev->devname[0] != '/') mdfd = open_dev(dv->devname); if (dev->devname[0] == '/' || mdfd < 0) mdfd = open_mddev(dv->devname, 1); But I thought you have a reason to use switch-case expression, so I kept switch-case. If you agree, I'll send v2 patch using if-expression. -- Best regards, Gi-Oh Kim TEL: 0176 2697 8962