Thank you for your reply. >On 02. 06. 23, 6:36, Shiji Yang wrote: >> From: Shiji Yang <yangshiji66@xxxxxx> >> >> Add a struct_group to around all members in struct ath_cycle_counters. >> It can help the compiler detect the intended bounds of the memcpy() and >> memset(). >> >> This patch fixes the following build warning: >> >> In function 'fortify_memset_chk', >> inlined from 'ath9k_ps_wakeup' at drivers/net/wireless/ath/ath9k/main.c:140:3: >> ./include/linux/fortify-string.h:314:25: error: call to '__write_overflow_field' declared with attribute warning: >> detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning] >> 314 | __write_overflow_field(p_size_field, size); >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> > >Hi, > >what compiler/version is this with? I'm using the mips gcc 12.3 (for arc ath79). It seems to be related to the compiler, and arm gcc 12.3 does not show compilation warnings. >> Signed-off-by: Shiji Yang <yangshiji66@xxxxxx> >> --- >> More discussion on: https://github.com/openwrt/openwrt/pull/12764 > >No "__write_overflow_field" there. Is this the right link? Yes. However, only few of the discussion here is related to compilation errors. The full log: make[4]: Entering directory '/home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24' make[5]: 'Kconfig.versions' is up to date. make[7]: 'Kconfig.versions' is up to date. make[8]: 'conf' is up to date. boolean symbol CRYPTO_LIB_ARC4 tested for 'm'? test forced to 'n' # # configuration written to .config # Building backport-include/backport/autoconf.h ... done. CC [M] /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k/main.o In file included from ./include/linux/string.h:253, from /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/backport-include/linux/string.h:3, from ./include/linux/bitmap.h:11, from ./include/linux/cpumask.h:12, from ./arch/mips/include/asm/processor.h:15, from ./arch/mips/include/asm/thread_info.h:16, from ./include/linux/thread_info.h:60, from ./include/asm-generic/current.h:5, from ./arch/mips/include/generated/asm/current.h:1, from ./include/linux/sched.h:12, from /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/backport-include/linux/sched.h:4, from ./include/linux/delay.h:23, from /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/backport-include/linux/delay.h:3, from /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k/main.c:18: In function 'fortify_memset_chk', inlined from 'ath9k_ps_wakeup' at /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k/main.c:140:3: ./include/linux/fortify-string.h:314:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning] 314 | __write_overflow_field(p_size_field, size); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors make[11]: *** [scripts/Makefile.build:250: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k/main.o] Error 1 make[10]: *** [scripts/Makefile.build:500: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k] Error 2 make[9]: *** [scripts/Makefile.build:500: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath] Error 2 make[8]: *** [scripts/Makefile.build:500: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless] Error 2 make[7]: *** [Makefile:2012: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24] Error 2 make[6]: *** [Makefile.build:13: modules] Error 2 make[5]: *** [Makefile.real:93: modules] Error 2 make[4]: *** [Makefile:121: modules] Error 2 make[4]: Leaving directory '/home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24' make[3]: *** [Makefile:401: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/.built] Error 2 make[3]: Leaving directory '/home/db/openwrt/package/kernel/mac80211' time: package/kernel/mac80211/regular/compile#4.94#0.95#21.75 ERROR: package/kernel/mac80211 failed to build (build variant: regular). make[2]: *** [package/Makefile:120: package/kernel/mac80211/compile] Error 1 make[2]: Leaving directory '/home/db/openwrt' make[1]: *** [package/Makefile:114: /home/db/openwrt/staging_dir/target-mips_24kc_musl/stamp/.package_compile] Error 2 make[1]: Leaving directory '/home/db/openwrt' make: *** [/home/db/openwrt/include/toplevel.mk:231: world] Error 2 >> --- >> drivers/net/wireless/ath/ath.h | 10 ++++++---- >> drivers/net/wireless/ath/ath5k/ani.c | 2 +- >> drivers/net/wireless/ath/ath5k/base.c | 4 ++-- >> drivers/net/wireless/ath/ath5k/mac80211-ops.c | 2 +- >> drivers/net/wireless/ath/ath9k/link.c | 2 +- >> drivers/net/wireless/ath/ath9k/main.c | 4 ++-- >> drivers/net/wireless/ath/hw.c | 2 +- >> 7 files changed, 14 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h >> index f02a308a9..4b42e401a 100644 >> --- a/drivers/net/wireless/ath/ath.h >> +++ b/drivers/net/wireless/ath/ath.h >> @@ -43,10 +43,12 @@ struct ath_ani { >> }; >> >> struct ath_cycle_counters { >> - u32 cycles; >> - u32 rx_busy; >> - u32 rx_frame; >> - u32 tx_frame; >> + struct_group(cnts, >> + u32 cycles; >> + u32 rx_busy; >> + u32 rx_frame; >> + u32 tx_frame; >> + ); > >This is horrid. Yes, I agree, but this can avoid making more changes. >> }; >> >> enum ath_device_state { >> diff --git a/drivers/net/wireless/ath/ath5k/ani.c b/drivers/net/wireless/ath/ath5k/ani.c >> index 850c608b4..fa95f0f0f 100644 >> --- a/drivers/net/wireless/ath/ath5k/ani.c >> +++ b/drivers/net/wireless/ath/ath5k/ani.c >> @@ -379,7 +379,7 @@ ath5k_hw_ani_get_listen_time(struct ath5k_hw *ah, struct ath5k_ani_state *as) >> spin_lock_bh(&common->cc_lock); >> >> ath_hw_cycle_counters_update(common); >> - memcpy(&as->last_cc, &common->cc_ani, sizeof(as->last_cc)); >> + memcpy(&as->last_cc.cnts, &common->cc_ani.cnts, sizeof(as->last_cc.cnts)); > >So is this. > >Care to elaborate why this is needed at all, provided we copy/zero a >whole structure? And describe it in the commit log, not in random >external sources. Thank you for the prompt. I will pay attention to the format of the patch next time. The compiler did not complain here. But in diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c index a4197c14f..8608a29a1 100644 --- a/drivers/net/wireless/ath/ath9k/main.c +++ b/drivers/net/wireless/ath/ath9k/main.c @@ -135,8 +135,8 @@ void ath9k_ps_wakeup(struct ath_softc *sc) if (power_mode != ATH9K_PM_AWAKE) { spin_lock(&common->cc_lock); ath_hw_cycle_counters_update(common); - memset(&common->cc_survey, 0, sizeof(common->cc_survey)); - memset(&common->cc_ani, 0, sizeof(common->cc_ani)); + memset(&common->cc_survey.cnts, 0, sizeof(common->cc_survey.cnts)); + memset(&common->cc_ani.cnts, 0, sizeof(common->cc_ani.cnts)); spin_unlock(&common->cc_lock); } Here, memset() is used to zero the entire structure. The compiler will only warn the second memset() `memset(&common->cc_ani, 0, sizeof(common->cc_ani));` However, `cc_survey` and `cc_survey` are the same structure. > >thanks, >-- >js >suse labs
