From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakob Koschel Date: Tue, 1 Mar 2022 18:40:04 +0100 Subject: [Intel-wired-lan] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr In-Reply-To: References: <20220228110822.491923-1-jakobkoschel@gmail.com> <20220228110822.491923-3-jakobkoschel@gmail.com> <2e4e95d6-f6c9-a188-e1cd-b1eae465562a@amd.com> Message-ID: <4B1AFAD9-C1B3-499C-945A-C259361ABA8C@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: intel-wired-lan@osuosl.org List-ID: > On 1. Mar 2022, at 18:36, Greg KH wrote: > > On Tue, Mar 01, 2022 at 12:28:15PM +0100, Jakob Koschel wrote: >> >> >>> On 1. Mar 2022, at 01:41, Linus Torvalds wrote: >>> >>> On Mon, Feb 28, 2022 at 1:47 PM Jakob Koschel 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. Very much appreciated! There will probably be some cases that do not match one of the pattern we already discussed and need separate attention. I was planning to start with one subsystem and adjust the coming ones according to the feedback gather there instead of posting all of them in one go. > > thanks, > > greg k-h - Jakob