On Tue, Mar 01, 2022 at 12:28:15PM +0100, Jakob Koschel wrote:
On 1. Mar 2022, at 01:41, Linus Torvalds torvalds@linux-foundation.org wrote:
On Mon, Feb 28, 2022 at 1:47 PM Jakob Koschel jakobkoschel@gmail.com wrote:
The goal of this is to get compiler warnings right? This would indeed be great.
Yes, so I don't mind having a one-time patch that has been gathered using some automated checker tool, but I don't think that works from a long-term maintenance perspective.
So if we have the basic rule being "don't use the loop iterator after the loop has finished, because it can cause all kinds of subtle issues", then in _addition_ to fixing the existing code paths that have this issue, I really would want to (a) get a compiler warning for future cases and (b) make it not actually _work_ for future cases.
Because otherwise it will just happen again.
Changing the list_for_each_entry() macro first will break all of those cases (e.g. the ones using 'list_entry_is_head()).
So I have no problems with breaking cases that we basically already have a patch for due to your automated tool. There were certainly more than a handful, but it didn't look _too_ bad to just make the rule be "don't use the iterator after the loop".
Of course, that's just based on that patch of yours. Maybe there are a ton of other cases that your patch didn't change, because they didn't match your trigger case, so I may just be overly optimistic here.
Based on the coccinelle script there are ~480 cases that need fixing in total. I'll now finish all of them and then split them by submodules as Greg suggested and repost a patch set per submodule. Sounds good?
Sounds good to me!
If you need help carving these up and maintaining them over time as different subsystem maintainers accept/ignore them, just let me know. Doing large patchsets like this can be tough without a lot of experience.
thanks,
greg k-h