[dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' statements

[dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' statements

[Markos Chandras]

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

Re: [dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' statements

[Anupam Kapoor]

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
>
>


-- 
In the beginning was the lambda, and the lambda was with Emacs, and Emacs
was the lambda.

Re: [dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' statements

[Markos Chandras]

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
>> 
>> 

-- 
SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg

Re: [dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' statements

[Anupam Kapoor]

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​

Re: [dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' statements

[Lu, Wenzhuo]

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.

Re: [dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' statements

[Thomas Monjalon]

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.

Re: [dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' statements

[Lu, Wenzhuo]

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.

Re: [dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' statements

[Bruce Richardson]

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

Re: [dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' statements

[Bruce Richardson]

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

Re: [dpdk-dev] [PATCH] e1000/base: Add missing braces to the 'if' statements

[Markos Chandras]

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.

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg