ixl: gcc build errors
ClosedPublic

Authored by rlibby on Jun 29 2017, 9:35 PM.

Details

Summary

Fix minor -Werror issues when building with gcc from -Wredundant-decls, -Wunused, -Wbool-operations.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
rlibby created this revision.Jun 29 2017, 9:35 PM

Ping. This is a NFC patch to fix compilation of ixl with gcc.

sbruno removed a reviewer: jfv.Jul 12 2017, 6:00 PM
erj edited edge metadata.EditedJul 12 2017, 6:45 PM

Is gcc requiring the additional const keywords?

Regardless, I think this patch is fine.

In D11414#239482, @erj wrote:

Is gcc requiring the additional const keywords?

Regardless, I think this patch is fine.

Not exactly. Regarding ixl_bcast_addr and ixl_fc_string, gcc actually complained about them being defined as static but then unused in some compilation units (-Werror=unused-variable, in e.g. if_ixl.c).

Declaring them const made gcc shut up about this, it doesn't complain about unused static const. If you like, we could also declare them extern and then define them in one of the source files (if_ixl.c? ixl_pf_main.c?), that would address the issue more directly. But I figured they ought to be const anyway. (The filter decls are then const propagation.)

Googling around, gcc bug 28901 activity suggests gcc may possibly change the warning behavior for unused static const, so maybe it does make sense to go the extern route anyway.

Example gcc 6.4 output below.

/usr/local/bin/gcc6 -c -O2 -frename-registers -pipe -fno-strict-aliasing  -g -nostdinc  -I. -I/usr/src/freebsd/sys -I/usr/src/freebsd/sys/contrib/libfdt -D_KERNEL -DHAVE_KERNEL_OPTION_HEADERS -include opt_global.h  -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -MD  -MF.depend.if_ixl.o -MTif_ixl.o -mcmodel=kernel -mno-red-zone -mno-mmx -mno-sse -msoft-float  -fno-asynchronous-unwind-tables -ffreestanding -fwrapv -fstack-protector -gdwarf-2 -Wall -Wredundant-decls -Wnested-externs -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual -Wundef -Wno-pointer-sign -Wno-format -Wmissing-include-dirs -fdiagnostics-show-option -Wno-unknown-pragmas -Wno-error=address -Wno-error=aggressive-loop-optimizations -Wno-error=array-bounds -Wno-error=attributes -Wno-error=cast-qual -Wno-error=enum-compare -Wno-error=inline -Wno-error=maybe-uninitialized -Wno-error=overflow -Wno-error=sequence-point -Wno-error=strict-overflow -Wno-error=unused-but-set-variable -Wno-error=misleading-indentation -Wno-error=nonnull-compare -Wno-error=shift-overflow -Wno-error=tautological-compare  -fno-common -fms-extensions -finline-limit=8000 --param inline-unit-growth=100 --param large-function-growth=1000  -std=iso9899:1999 -Werror  /usr/src/freebsd/sys/dev/ixl/if_ixl.c -I/usr/src/freebsd/sys/dev/ixl
In file included from /usr/src/freebsd/sys/dev/ixl/ixl.h:106:0,
                 from /usr/src/freebsd/sys/dev/ixl/if_ixl.c:35:
/usr/src/freebsd/sys/dev/ixl/i40e_prototype.h:541:23: error: redundant redeclaration of 'i40e_blink_phy_link_led' [-Werror=redundant-decls]
 enum i40e_status_code i40e_blink_phy_link_led(struct i40e_hw *hw,
                       ^~~~~~~~~~~~~~~~~~~~~~~
/usr/src/freebsd/sys/dev/ixl/i40e_prototype.h:101:23: note: previous declaration of 'i40e_blink_phy_link_led' was here
 enum i40e_status_code i40e_blink_phy_link_led(struct i40e_hw *hw,
                       ^~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/src/freebsd/sys/dev/ixl/if_ixl.c:36:0:
/usr/src/freebsd/sys/dev/ixl/ixl_pf.h:171:14: error: 'ixl_fc_string' defined but not used [-Werror=unused-variable]
 static char *ixl_fc_string[6] = {
              ^~~~~~~~~~~~~
In file included from /usr/src/freebsd/sys/dev/ixl/if_ixl.c:35:0:
/usr/src/freebsd/sys/dev/ixl/ixl.h:667:16: error: 'ixl_bcast_addr' defined but not used [-Werror=unused-variable]
 static uint8_t ixl_bcast_addr[ETHER_ADDR_LEN] =
                ^~~~~~~~~~~~~~
cc1: all warnings being treated as errors
*** Error code 1

Stop.
make[2]: stopped in /usr/obj/gcc6/usr/src/freebsd/sys/GENERIC
*** Error code 1

Stop.
make[1]: stopped in /usr/src/freebsd
*** Error code 1

Stop.
make: stopped in /usr/src/freebsd
erj requested changes to this revision.Jul 12 2017, 10:36 PM

@rlibby , after reading the linked bug report, I think it would be a good idea to go the extern route. Could you regenerate the patch and define ixl_bcast_addr and ixl_fc_string in ixl_pf_main.c? (The latter would only ever get used in the PF driver; only the PF driver uses the former). Or just commit everything but the changes to those two, and I could make those changes.

And yeah, making them const is correct.

This revision now requires changes to proceed.Jul 12 2017, 10:36 PM
rlibby updated this revision to Diff 30714.Jul 13 2017, 8:37 AM
rlibby edited edge metadata.

erj feedback: also declare objects extern and define only once

I also noticed that malloc type M_IXL was being multiply defined, and I
applied the same change to it.

erj feedback: also declare objects extern and define only once

I also noticed that malloc type M_IXL was being multiply defined, and I
applied the same change to it.

@erj, you may want to check this before/after the patch and verify the problem/fix. I think it will should show multiple ixl malloc types before the patch, and just one after.
vmstat -m | grep ixl

erj accepted this revision.Jul 13 2017, 8:40 PM

There's still only one entry before the patch. But this looks more correct.

This revision is now accepted and ready to land.Jul 13 2017, 8:40 PM
This revision was automatically updated to reflect the committed changes.