On Tue, 2018-10-23 at 12:58 +0200, Gustavo A. R. Silva wrote: > On 10/23/18 10:59 AM, Gustavo A. R. Silva wrote: > > > > On 10/23/18 9:01 AM, Johannes Berg wrote: > > > On Tue, 2018-10-23 at 02:13 +0200, Gustavo A. R. Silva wrote: > > > > In preparation to enabling -Wimplicit-fallthrough, mark switch cases > > > > where we are expecting to fall through. > > > > > > > > Warning level 3 was used: -Wimplicit-fallthrough=3 > > > > > > > > This code was not tested and GCC 7.2.0 was used to compile it. > > > > > > Look, I'm not going to make this any clearer: I'm not applying patches > > > like that where you've invested no effort whatsoever on verifying that > > > they're correct. > > > > > > > How do you suggest me to verify that every part is correct in this type > > of patches? > > > > BTW... I'm under the impression you think that I don't even look at > the code. Is that correct?
That's what your commit log looks like, yes. This is your full commit log: In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Warning level 3 was used: -Wimplicit-fallthrough=3 This code was not tested and GCC 7.2.0 was used to compile it. For all I know, you could've run spatch to add the comments wherever there was no break, and then compiled it once. > I've been working on this for quite a while, and in every case I try to > understand the code in terms of the context in which every warning is > reported. That's great. > Here is a bug I found yesterday at > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > > 5690 case WLAN_CIPHER_SUITE_CCMP: > 5691 key->flags |= IEEE80211_KEY_FLAG_SW_MGMT_TX; > 5692 break; > 5693 case WLAN_CIPHER_SUITE_TKIP: > 5694 key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC; > 5695 default: > 5696 return -EOPNOTSUPP; > 5697 } Indeed, that looks like a bug, although kinda benign since it just means TKIP will always use software crypto and TKIP is slow anyway ;-) > I do this analysis for every warning. Now, when I say I haven't tested the > code > is because I don't have any log as evidence for anything. Not that I haven't > put > any effort on trying to understand it and its context. When I started > working on > this task there were more than 2000 of these issues, now there are around 600 > left. > > I have fixed many bugs on the way, so a good amount of work is being invested > on > this, and it's paying off. :) :-) > Now, let me ask you this question: > > It would be easier for you to review this patch if I turn it into a series? > > I can do that without a problem. I'd be happy if you were to actually just mention that you've looked at them, and found them to be expected fall throughs. I'll still review them, but without that information I feel like I'm doing the first round of reviews this code ever got. johannes
