| Message ID | 20160623092552.30932-1-mchandras@suse.de (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Bruce Richardson |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id E335CC46E; Thu, 23 Jun 2016 11:26:02 +0200 (CEST) Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by dpdk.org (Postfix) with ESMTP id EEDF6C412 for <dev@dpdk.org>; Thu, 23 Jun 2016 11:26:00 +0200 (CEST) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 7DE1CAC6E for <dev@dpdk.org>; Thu, 23 Jun 2016 09:26:00 +0000 (UTC) From: Markos Chandras <mchandras@suse.de> To: dev@dpdk.org Cc: Markos Chandras <mchandras@suse.de> Date: Thu, 23 Jun 2016 10:25:52 +0100 Message-Id: <20160623092552.30932-1-mchandras@suse.de> X-Mailer: git-send-email 2.8.4 Subject: [dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' statements X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Markos Chandras
June 23, 2016, 9:25 a.m. UTC
Add the missing braces to the 'if' statements to fix the misleading
identation. This also fixes the following build errors when building
with gcc >= 6:
drivers/net/e1000/base/e1000_phy.c:4156:2:
error: this 'if' clause does not guard... [-Werror=misleading-indentation]
if (locked)
^~
drivers/net/e1000/base/e1000_phy.c:4158:3:
note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if'
if (!ready)
^~
drivers/net/e1000/base/e1000_phy.c: In function 'e1000_write_phy_reg_mphy':
drivers/net/e1000/base/e1000_phy.c:4221:2:
error: this 'if' clause does not guard... [-Werror=misleading-indentation]
if (locked)
^~
drivers/net/e1000/base/e1000_phy.c:4223:3:
note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if'
if (!ready)
^~
Signed-off-by: Markos Chandras <mchandras@suse.de>
---
drivers/net/e1000/base/e1000_phy.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
Comments
hi markos, please see : cba50f6be0db9efdf694dcf4bce4a6945a275182, which should already fix this. -- thanks anupam On Thu, Jun 23, 2016 at 2:55 PM, Markos Chandras <mchandras@suse.de> wrote: > Add the missing braces to the 'if' statements to fix the misleading > identation. This also fixes the following build errors when building > with gcc >= 6: > > drivers/net/e1000/base/e1000_phy.c:4156:2: > error: this 'if' clause does not guard... [-Werror=misleading-indentation] > if (locked) > ^~ > > drivers/net/e1000/base/e1000_phy.c:4158:3: > note: ...this statement, but the latter is misleadingly indented as if it > is guarded by the 'if' > if (!ready) > ^~ > > drivers/net/e1000/base/e1000_phy.c: In function 'e1000_write_phy_reg_mphy': > drivers/net/e1000/base/e1000_phy.c:4221:2: > error: this 'if' clause does not guard... [-Werror=misleading-indentation] > if (locked) > ^~ > > drivers/net/e1000/base/e1000_phy.c:4223:3: > note: ...this statement, but the latter is misleadingly indented as if it > is guarded by the 'if' > if (!ready) > ^~ > > Signed-off-by: Markos Chandras <mchandras@suse.de> > --- > drivers/net/e1000/base/e1000_phy.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/e1000/base/e1000_phy.c > b/drivers/net/e1000/base/e1000_phy.c > index d43b7ce..33f478b 100644 > --- a/drivers/net/e1000/base/e1000_phy.c > +++ b/drivers/net/e1000/base/e1000_phy.c > @@ -4153,12 +4153,13 @@ s32 e1000_read_phy_reg_mphy(struct e1000_hw *hw, > u32 address, u32 *data) > *data = E1000_READ_REG(hw, E1000_MPHY_DATA); > > /* Disable access to mPHY if it was originally disabled */ > - if (locked) > + if (locked) { > ready = e1000_is_mphy_ready(hw); > if (!ready) > return -E1000_ERR_PHY; > E1000_WRITE_REG(hw, E1000_MPHY_ADDR_CTRL, > E1000_MPHY_DIS_ACCESS); > + } > > return E1000_SUCCESS; > } > @@ -4218,12 +4219,13 @@ s32 e1000_write_phy_reg_mphy(struct e1000_hw *hw, > u32 address, u32 data, > E1000_WRITE_REG(hw, E1000_MPHY_DATA, data); > > /* Disable access to mPHY if it was originally disabled */ > - if (locked) > + if (locked) { > ready = e1000_is_mphy_ready(hw); > if (!ready) > return -E1000_ERR_PHY; > E1000_WRITE_REG(hw, E1000_MPHY_ADDR_CTRL, > E1000_MPHY_DIS_ACCESS); > + } > > return E1000_SUCCESS; > } > -- > 2.8.4 > >
Hi Anupam, I have seen your commit, but my patch fixes a different file (although the fix is similar). Am I missing something? On 2016-06-23 11:26, Anupam Kapoor wrote: > hi markos, > > please see : cba50f6be0db9efdf694dcf4bce4a6945a275182, which should > already > fix this. > > -- > thanks > anupam > > > On Thu, Jun 23, 2016 at 2:55 PM, Markos Chandras <mchandras@suse.de> > wrote: > >> Add the missing braces to the 'if' statements to fix the misleading >> identation. This also fixes the following build errors when building >> with gcc >= 6: >> >> drivers/net/e1000/base/e1000_phy.c:4156:2: >> error: this 'if' clause does not guard... >> [-Werror=misleading-indentation] >> if (locked) >> ^~ >> >> drivers/net/e1000/base/e1000_phy.c:4158:3: >> note: ...this statement, but the latter is misleadingly indented as if >> it >> is guarded by the 'if' >> if (!ready) >> ^~ >> >> drivers/net/e1000/base/e1000_phy.c: In function >> 'e1000_write_phy_reg_mphy': >> drivers/net/e1000/base/e1000_phy.c:4221:2: >> error: this 'if' clause does not guard... >> [-Werror=misleading-indentation] >> if (locked) >> ^~ >> >> drivers/net/e1000/base/e1000_phy.c:4223:3: >> note: ...this statement, but the latter is misleadingly indented as if >> it >> is guarded by the 'if' >> if (!ready) >> ^~ >> >> Signed-off-by: Markos Chandras <mchandras@suse.de> >> --- >> drivers/net/e1000/base/e1000_phy.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/e1000/base/e1000_phy.c >> b/drivers/net/e1000/base/e1000_phy.c >> index d43b7ce..33f478b 100644 >> --- a/drivers/net/e1000/base/e1000_phy.c >> +++ b/drivers/net/e1000/base/e1000_phy.c >> @@ -4153,12 +4153,13 @@ s32 e1000_read_phy_reg_mphy(struct e1000_hw >> *hw, >> u32 address, u32 *data) >> *data = E1000_READ_REG(hw, E1000_MPHY_DATA); >> >> /* Disable access to mPHY if it was originally disabled */ >> - if (locked) >> + if (locked) { >> ready = e1000_is_mphy_ready(hw); >> if (!ready) >> return -E1000_ERR_PHY; >> E1000_WRITE_REG(hw, E1000_MPHY_ADDR_CTRL, >> E1000_MPHY_DIS_ACCESS); >> + } >> >> return E1000_SUCCESS; >> } >> @@ -4218,12 +4219,13 @@ s32 e1000_write_phy_reg_mphy(struct e1000_hw >> *hw, >> u32 address, u32 data, >> E1000_WRITE_REG(hw, E1000_MPHY_DATA, data); >> >> /* Disable access to mPHY if it was originally disabled */ >> - if (locked) >> + if (locked) { >> ready = e1000_is_mphy_ready(hw); >> if (!ready) >> return -E1000_ERR_PHY; >> E1000_WRITE_REG(hw, E1000_MPHY_ADDR_CTRL, >> E1000_MPHY_DIS_ACCESS); >> + } >> >> return E1000_SUCCESS; >> } >> -- >> 2.8.4 >> >>
On Thu, Jun 23, 2016 at 4:04 PM, Markos Chandras <mchandras@suse.de> wrote: > I have seen your commit, but my patch fixes a different file (although the > fix is similar). > > Am I missing something? > ah no. my bad. sorry about that. -- thanks anupam
Hi Markos, > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Markos Chandras > Sent: Thursday, June 23, 2016 5:26 PM > To: dev@dpdk.org > Cc: Markos Chandras > Subject: [dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' > statements > > Add the missing braces to the 'if' statements to fix the misleading identation. > This also fixes the following build errors when building with gcc >= 6: > > drivers/net/e1000/base/e1000_phy.c:4156:2: > error: this 'if' clause does not guard... [-Werror=misleading-indentation] if > (locked) ^~ > > drivers/net/e1000/base/e1000_phy.c:4158:3: > note: ...this statement, but the latter is misleadingly indented as if it is guarded > by the 'if' > if (!ready) > ^~ > > drivers/net/e1000/base/e1000_phy.c: In function 'e1000_write_phy_reg_mphy': > drivers/net/e1000/base/e1000_phy.c:4221:2: > error: this 'if' clause does not guard... [-Werror=misleading-indentation] if > (locked) ^~ > > drivers/net/e1000/base/e1000_phy.c:4223:3: > note: ...this statement, but the latter is misleadingly indented as if it is guarded > by the 'if' > if (!ready) > ^~ > > Signed-off-by: Markos Chandras <mchandras@suse.de> Thanks for this patch. But normally the code in the base directory is synced from the kernel driver. So we don't change it if there's no critical issue. It's easy for us to maintain it. Thanks.
2016-06-24 00:43, Lu, Wenzhuo:
> Thanks for this patch. But normally the code in the base directory is synced from the kernel driver. So we don't change it if there's no critical issue. It's easy for us to maintain it. Thanks.
I think a build error is critical enough.
Hi Thomas, Markos, > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Friday, June 24, 2016 3:13 PM > To: Lu, Wenzhuo > Cc: dev@dpdk.org; Markos Chandras > Subject: Re: [dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' > statements > > 2016-06-24 00:43, Lu, Wenzhuo: > > Thanks for this patch. But normally the code in the base directory is synced > from the kernel driver. So we don't change it if there's no critical issue. It's easy > for us to maintain it. Thanks. > > I think a build error is critical enough. OK. The change itself looks fine to me.
On Thu, Jun 23, 2016 at 10:25:52AM +0100, Markos Chandras wrote: > Add the missing braces to the 'if' statements to fix the misleading > identation. This also fixes the following build errors when building > with gcc >= 6: > > drivers/net/e1000/base/e1000_phy.c:4156:2: > error: this 'if' clause does not guard... [-Werror=misleading-indentation] > if (locked) > ^~ > > drivers/net/e1000/base/e1000_phy.c:4158:3: > note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if' > if (!ready) > ^~ > > drivers/net/e1000/base/e1000_phy.c: In function 'e1000_write_phy_reg_mphy': > drivers/net/e1000/base/e1000_phy.c:4221:2: > error: this 'if' clause does not guard... [-Werror=misleading-indentation] > if (locked) > ^~ > > drivers/net/e1000/base/e1000_phy.c:4223:3: > note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if' > if (!ready) > ^~ > > Signed-off-by: Markos Chandras <mchandras@suse.de> > --- Any particular compiler flags needed to reproduce this issue? Compiling with gcc6.1 I don't see any errors reported. /Bruce
On Fri, Jun 24, 2016 at 08:31:05AM +0000, Lu, Wenzhuo wrote: > Hi Thomas, Markos, > > > > -----Original Message----- > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > Sent: Friday, June 24, 2016 3:13 PM > > To: Lu, Wenzhuo > > Cc: dev@dpdk.org; Markos Chandras > > Subject: Re: [dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' > > statements > > > > 2016-06-24 00:43, Lu, Wenzhuo: > > > Thanks for this patch. But normally the code in the base directory is synced > > from the kernel driver. So we don't change it if there's no critical issue. It's easy > > for us to maintain it. Thanks. > > > > I think a build error is critical enough. > OK. The change itself looks fine to me. Applied to dpdk-next-net/rel_16_07 /Bruce
Hi Bruce, On 06/27/2016 03:39 PM, Bruce Richardson wrote: > On Thu, Jun 23, 2016 at 10:25:52AM +0100, Markos Chandras wrote: >> Add the missing braces to the 'if' statements to fix the misleading >> identation. This also fixes the following build errors when building >> with gcc >= 6: >> >> drivers/net/e1000/base/e1000_phy.c:4156:2: >> error: this 'if' clause does not guard... [-Werror=misleading-indentation] >> if (locked) >> ^~ >> >> drivers/net/e1000/base/e1000_phy.c:4158:3: >> note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if' >> if (!ready) >> ^~ >> >> drivers/net/e1000/base/e1000_phy.c: In function 'e1000_write_phy_reg_mphy': >> drivers/net/e1000/base/e1000_phy.c:4221:2: >> error: this 'if' clause does not guard... [-Werror=misleading-indentation] >> if (locked) >> ^~ >> >> drivers/net/e1000/base/e1000_phy.c:4223:3: >> note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if' >> if (!ready) >> ^~ >> >> Signed-off-by: Markos Chandras <mchandras@suse.de> >> --- > > Any particular compiler flags needed to reproduce this issue? Compiling with > gcc6.1 I don't see any errors reported. > > /Bruce > I only have the log from the 2.2.0 + gcc-6 build so here is the line gcc -Wp,-MD,./.e1000_phy.o.d.tmp -m64 -pthread -fPIC -march=core2 -DRTE_MACHINE_CPUFLAG_SSE -DRTE_MACHINE_CPUFLAG_SSE2 -DRTE_MACHINE_CPUFLAG_SSE3 -DRTE_MACHINE_CPUFLAG_SSSE3 -DRTE_COMPILE_TIME_CPUFLAGS =RTE_CPUFLAG_SSE,RTE_CPUFLAG_SSE2,RTE_CPUFLAG_SSE3,RTE_CPUFLAG_SSSE3 -I/home/abuild/rpmbuild/BUILD/dpdk-2.2.0/x86_64-native-linuxapp-gcc/include -include /home/abuild/rpmbuild/BUILD/dpdk-2.2.0/x86_64-native-linuxapp-gcc/include/rte_config.h -O3 -W -Wall -Werror -Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wpointer-arith -Wcast-align -Wnested-externs -Wcast-qual -Wformat-nonli teral -Wformat-security -Wundef -Wwrite-strings -Wno-uninitialized -Wno-unused-parameter -Wno-unused-variable -fmessage-length=0 -grecord-gcc-switches -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector-strong -funw ind-tables -fasynchronous-unwind-tables -g -Wformat -fPIC -Wno-error=array-bounds -o e1000_phy.o -c /home/abuild/rpmbuild/BUILD/dpdk-2.2.0/drivers/net/e1000/base/e1000_phy.c But next time I will make sure I will add the command line that causes the problem in the comment section of the patch as well.
diff --git a/drivers/net/e1000/base/e1000_phy.c b/drivers/net/e1000/base/e1000_phy.c index d43b7ce..33f478b 100644 --- a/drivers/net/e1000/base/e1000_phy.c +++ b/drivers/net/e1000/base/e1000_phy.c @@ -4153,12 +4153,13 @@ s32 e1000_read_phy_reg_mphy(struct e1000_hw *hw, u32 address, u32 *data) *data = E1000_READ_REG(hw, E1000_MPHY_DATA); /* Disable access to mPHY if it was originally disabled */ - if (locked) + if (locked) { ready = e1000_is_mphy_ready(hw); if (!ready) return -E1000_ERR_PHY; E1000_WRITE_REG(hw, E1000_MPHY_ADDR_CTRL, E1000_MPHY_DIS_ACCESS); + } return E1000_SUCCESS; } @@ -4218,12 +4219,13 @@ s32 e1000_write_phy_reg_mphy(struct e1000_hw *hw, u32 address, u32 data, E1000_WRITE_REG(hw, E1000_MPHY_DATA, data); /* Disable access to mPHY if it was originally disabled */ - if (locked) + if (locked) { ready = e1000_is_mphy_ready(hw); if (!ready) return -E1000_ERR_PHY; E1000_WRITE_REG(hw, E1000_MPHY_ADDR_CTRL, E1000_MPHY_DIS_ACCESS); + } return E1000_SUCCESS; }