[dpdk-dev,v2,2/7] drivers/net/e1000: Fix missing brackets

Message ID 56F38F26.1030707@redhat.com (mailing list archive)
State Not Applicable, archived
Headers

Commit Message

Panu Matilainen March 24, 2016, 6:54 a.m. UTC
  On 03/24/2016 02:35 AM, Lu, Wenzhuo wrote:
> Hi Thomas, Aaron,
>>> Wenzhuo, do you agree to fix the base driver here?
> Honestly I find the logic has some problem and maybe more changes needed. I'm talking with our kernel driver contactors to make sure they have no concern about  my idea.
> And Aaron, do we really need to fix this code? For I find this function is not called. So it has no real impact to us. Could we just wait until kernel driver fixes it and leverage the new version? Thanks.

It breaks the build with gcc >= 6.0 due to -Werror so yes we need to fix 
it, one way or the other.

As spelled out in an earlier mail 
(http://dpdk.org/ml/archives/dev/2016-March/034290.html), for somebody 
building with gcc >= 6.0 the options basically are:
1) disable the driver entirely
2) build with -Wno-error
3) paper over it with local warning disablers
4) patch the thing locally

If the offending function really is not used at all in DPDK context then 
3) is an entirely valid option for an upstream solution, ie something 
like (untested)



	- Panu -
  

Comments

Thomas Monjalon March 30, 2016, 10:51 a.m. UTC | #1
2016-03-24 08:54, Panu Matilainen:
> --- a/drivers/net/e1000/Makefile
> +++ b/drivers/net/e1000/Makefile
> @@ -54,6 +54,9 @@ else
>   #
>   CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
>   CFLAGS_BASE_DRIVER += -Wno-unused-variable
> +ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
> +CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
> +endif

Aaron, have you tested this solution?
Are you going to provide a v3?
  
Aaron Conole March 30, 2016, 1:14 p.m. UTC | #2
Thomas Monjalon <thomas.monjalon@6wind.com> writes:

> 2016-03-24 08:54, Panu Matilainen:
>> --- a/drivers/net/e1000/Makefile
>> +++ b/drivers/net/e1000/Makefile
>> @@ -54,6 +54,9 @@ else
>>   #
>>   CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
>>   CFLAGS_BASE_DRIVER += -Wno-unused-variable
>> +ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
>> +CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
>> +endif
>
> Aaron, have you tested this solution?
> Are you going to provide a v3?

I haven't yet tested this solution, but if folks are that opposed to
changing the code, then I will test it and post a v3 of this particular
patch in the series. 

Thanks so much for the reviews and time on this (Panu AND Thomas :-)),
-Aaron
  

Patch

--- a/drivers/net/e1000/Makefile
+++ b/drivers/net/e1000/Makefile
@@ -54,6 +54,9 @@  else
  #
  CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
  CFLAGS_BASE_DRIVER += -Wno-unused-variable
+ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
+CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
+endif
  endif

  #