| Message ID | 20180524225928.GA19570@embeddedor.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
"Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote: > In preparation to enabling -Wimplicit-fallthrough, mark switch cases > where we are expecting to fall through. > > Notice that in this particular case, I replaced "pass through" with > a proper "fall through" comment, which is what GCC is expecting > to find. > > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org> Patch applied to ath-next branch of ath.git, thanks. f1d270ae10ff ath10k: htt_tx: mark expected switch fall-throughs
(replying to an old thread but renaming it) Kalle Valo <kvalo@codeaurora.org> writes: > "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote: > >> In preparation to enabling -Wimplicit-fallthrough, mark switch cases >> where we are expecting to fall through. >> >> Notice that in this particular case, I replaced "pass through" with >> a proper "fall through" comment, which is what GCC is expecting >> to find. >> >> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> >> Signed-off-by: Kalle Valo <kvalo@codeaurora.org> > > Patch applied to ath-next branch of ath.git, thanks. > > f1d270ae10ff ath10k: htt_tx: mark expected switch fall-throughs Gustavo, I enabled W=1 on my ath10k build checks and it took me a while to figure out why GCC was warning about fall through annotations missing even I knew you had fixed them. Finally I figured out that the reason was ccache, which I need because I work with different branches and need to recompile the kernel quite often. If the plan is to enable -Wimplicit-fallthrough by default in the kernel IMHO this might become an issue, as otherwise people using ccache start seeing lots of invalid warnings. Apparently CCACHE_COMMENTS=1 will fix that but my version of ccache doesn't support it, and how would everyone learn that trick anyway? Or maybe CCACHE_COMMENTS can enabled through kernel Makefile?
Hi Kalle, On 2/16/19 5:21 AM, Kalle Valo wrote: > (replying to an old thread but renaming it) > > Kalle Valo <kvalo@codeaurora.org> writes: > >> "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote: >> >>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases >>> where we are expecting to fall through. >>> >>> Notice that in this particular case, I replaced "pass through" with >>> a proper "fall through" comment, which is what GCC is expecting >>> to find. >>> >>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> >>> Signed-off-by: Kalle Valo <kvalo@codeaurora.org> >> >> Patch applied to ath-next branch of ath.git, thanks. >> >> f1d270ae10ff ath10k: htt_tx: mark expected switch fall-throughs > > Gustavo, I enabled W=1 on my ath10k build checks and it took me a while > to figure out why GCC was warning about fall through annotations missing > even I knew you had fixed them. Finally I figured out that the reason > was ccache, which I need because I work with different branches and need > to recompile the kernel quite often. > > If the plan is to enable -Wimplicit-fallthrough by default in the kernel > IMHO this might become an issue, as otherwise people using ccache start > seeing lots of invalid warnings. Apparently CCACHE_COMMENTS=1 will fix > that but my version of ccache doesn't support it, and how would everyone > learn that trick anyway? Or maybe CCACHE_COMMENTS can enabled through > kernel Makefile? > Can you share with me the warning messages you get? I just see the following warnings with linux-next: $ make CC="ccache gcc" W=1 drivers/net/wireless/ath/ath10k/htt_tx.o CC [M] drivers/net/wireless/ath/ath10k/htt_tx.o In file included from drivers/net/wireless/ath/ath10k/htt_tx.c:19: drivers/net/wireless/ath/ath10k/htt.h:1727:1: warning: alignment 1 of ‘struct ath10k_htt_txbuf_32’ is less than 4 [-Wpacked-not-aligned] } __packed; ^ drivers/net/wireless/ath/ath10k/htt.h:1734:1: warning: alignment 1 of ‘struct ath10k_htt_txbuf_64’ is less than 4 [-Wpacked-not-aligned] } __packed; ^ In my Makefile I have: KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough=3,) Thanks -- Gustavo
diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c index 5d8b97a..89157c5 100644 --- a/drivers/net/wireless/ath/ath10k/htt_tx.c +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c @@ -1202,7 +1202,7 @@ static int ath10k_htt_tx_32(struct ath10k_htt *htt, case ATH10K_HW_TXRX_RAW: case ATH10K_HW_TXRX_NATIVE_WIFI: flags0 |= HTT_DATA_TX_DESC_FLAGS0_MAC_HDR_PRESENT; - /* pass through */ + /* fall through */ case ATH10K_HW_TXRX_ETHERNET: if (ar->hw_params.continuous_frag_desc) { ext_desc_t = htt->frag_desc.vaddr_desc_32; @@ -1404,7 +1404,7 @@ static int ath10k_htt_tx_64(struct ath10k_htt *htt, case ATH10K_HW_TXRX_RAW: case ATH10K_HW_TXRX_NATIVE_WIFI: flags0 |= HTT_DATA_TX_DESC_FLAGS0_MAC_HDR_PRESENT; - /* pass through */ + /* fall through */ case ATH10K_HW_TXRX_ETHERNET: if (ar->hw_params.continuous_frag_desc) { ext_desc_t = htt->frag_desc.vaddr_desc_64;
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Notice that in this particular case, I replaced "pass through" with a proper "fall through" comment, which is what GCC is expecting to find. Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> --- drivers/net/wireless/ath/ath10k/htt_tx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)