Fix minor -Werror issues when building with gcc from -Wredundant-decls, -Wunused, -Wbool-operations.
Details
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
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
@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.
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