Page MenuHomeFreeBSD

ixl: gcc build errors
ClosedPublic

Authored by rlibby on Jun 29 2017, 9:35 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 25, 12:52 PM
Unknown Object (File)
Sun, Nov 3, 1:01 PM
Unknown Object (File)
Sun, Nov 3, 1:01 PM
Unknown Object (File)
Sun, Nov 3, 1:00 PM
Unknown Object (File)
Sun, Nov 3, 1:00 PM
Unknown Object (File)
Sun, Nov 3, 12:41 PM
Unknown Object (File)
Sat, Nov 2, 8:19 AM
Unknown Object (File)
Oct 5 2024, 7:02 PM
Subscribers

Details

Summary

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

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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

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

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.