From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 29DBAC433EF for ; Mon, 28 Feb 2022 22:29:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231383AbiB1W3p (ORCPT ); Mon, 28 Feb 2022 17:29:45 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59526 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229625AbiB1W3n (ORCPT ); Mon, 28 Feb 2022 17:29:43 -0500 Received: from bedivere.hansenpartnership.com (bedivere.hansenpartnership.com [96.44.175.130]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8125CEDF14 for ; Mon, 28 Feb 2022 14:29:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=hansenpartnership.com; s=20151216; t=1646087344; bh=gnitHaDoKtFYFmdfHx7ZmUK+2K99mC4OdMWiGF3yqts=; h=Message-ID:Subject:From:To:Date:In-Reply-To:References:From; b=aDv+zUJMVLbpHnTobs88HMeKXt8e/Fsm3NyltqbtXmut8dDpKPmgZQHvU7CQAhpxT x8yyOH8cKkqI9wCCVd7mWgGGPtOdFvurEvTcoJzgoG8F+eqVyUcxI30U1Z3UyKmv+s JFRv6cmaJ/MAgdj571KO+Pve8Zec5bTLqNmzMPjU= Received: from localhost (localhost [127.0.0.1]) by bedivere.hansenpartnership.com (Postfix) with ESMTP id 28A0B1280EAE; Mon, 28 Feb 2022 17:29:04 -0500 (EST) Received: from bedivere.hansenpartnership.com ([127.0.0.1]) by localhost (bedivere.hansenpartnership.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id WDTR3R1x8W6K; Mon, 28 Feb 2022 17:29:04 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=hansenpartnership.com; s=20151216; t=1646087343; bh=gnitHaDoKtFYFmdfHx7ZmUK+2K99mC4OdMWiGF3yqts=; h=Message-ID:Subject:From:To:Date:In-Reply-To:References:From; b=u33hwQnb0VqdOXP8ulufSc5IRuZCZipVBGZNcGF9+k1EE0TmKFXpcGOMGd4fMwEgO 9syQhDBoNk9Z7GUqfbwA47rALQtXFHUxFeSSP/G0sxjEsJWs9EMuAmppU6twhzQzk+ fAgT3F3AJIbVTwM2ufsCoBYKhhHBSRhleqdyR2l4= Received: from jarvis.int.hansenpartnership.com (unknown [IPv6:2601:5c4:4300:c551::527]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits)) (No client certificate requested) by bedivere.hansenpartnership.com (Postfix) with ESMTPSA id C21DB1280D34; Mon, 28 Feb 2022 17:28:59 -0500 (EST) Message-ID: <73fa82a20910c06784be2352a655acc59e9942ea.camel@HansenPartnership.com> Subject: Re: [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr From: James Bottomley To: Mike Rapoport , Christian =?ISO-8859-1?Q?K=F6nig?= , Linus Torvalds Cc: Jakob Koschel , alsa-devel@alsa-project.org, linux-aspeed@lists.ozlabs.org, "Gustavo A. R. Silva" , linux-iio@vger.kernel.org, nouveau@lists.freedesktop.org, Rasmus Villemoes , dri-devel , Cristiano Giuffrida , amd-gfx list , samba-technical@lists.samba.org, linux1394-devel@lists.sourceforge.net, drbd-dev@lists.linbit.com, linux-arch , CIFS , KVM list , linux-scsi , linux-rdma , linux-staging@lists.linux.dev, "Bos, H.J." , Jason Gunthorpe , intel-wired-lan@lists.osuosl.org, kgdb-bugreport@lists.sourceforge.net, bcm-kernel-feedback-list@broadcom.com, Dan Carpenter , Linux Media Mailing List , Kees Cook , Arnd Bergman , Linux PM , intel-gfx , Brian Johannesmeyer , Nathan Chancellor , linux-fsdevel , Christophe JAILLET , v9fs-developer@lists.sourceforge.net, linux-tegra , Thomas Gleixner , Andy Shevchenko , Linux ARM , linux-sgx@vger.kernel.org, linux-block , Netdev , linux-usb@vger.kernel.org, linux-wireless , Linux Kernel Mailing List , Linux F2FS Dev Mailing List , tipc-discussion@lists.sourceforge.net, Linux Crypto Mailing List , dma , linux-mediatek@lists.infradead.org, Andrew Morton , linuxppc-dev Date: Mon, 28 Feb 2022 17:28:58 -0500 In-Reply-To: <7D0C2A5D-500E-4F38-AD0C-A76E132A390E@kernel.org> References: <20220228110822.491923-1-jakobkoschel@gmail.com> <20220228110822.491923-3-jakobkoschel@gmail.com> <2e4e95d6-f6c9-a188-e1cd-b1eae465562a@amd.com> <282f0f8d-f491-26fc-6ae0-604b367a5a1a@amd.com> <7D0C2A5D-500E-4F38-AD0C-A76E132A390E@kernel.org> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.34.4 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Mon, 2022-02-28 at 23:59 +0200, Mike Rapoport wrote: > > On February 28, 2022 10:42:53 PM GMT+02:00, James Bottomley < > James.Bottomley@HansenPartnership.com> wrote: > > On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote: [...] > > > > I do wish we could actually poison the 'pos' value after the > > > > loop somehow - but clearly the "might be uninitialized" I was > > > > hoping for isn't the way to do it. > > > > > > > > Anybody have any ideas? > > > > > > I think we should look at the use cases why code is touching > > > (pos) after the loop. > > > > > > Just from skimming over the patches to change this and experience > > > with the drivers/subsystems I help to maintain I think the > > > primary pattern looks something like this: > > > > > > list_for_each_entry(entry, head, member) { > > > if (some_condition_checking(entry)) > > > break; > > > } > > > do_something_with(entry); > > > > Actually, we usually have a check to see if the loop found > > anything, but in that case it should something like > > > > if (list_entry_is_head(entry, head, member)) { > > return with error; > > } > > do_somethin_with(entry); > > > > Suffice? The list_entry_is_head() macro is designed to cope with > > the bogus entry on head problem. > > Won't suffice because the end goal of this work is to limit scope of > entry only to loop. Hence the need for additional variable. Well, yes, but my objection is more to the size of churn than the desire to do loop local. I'm not even sure loop local is possible, because it's always annoyed me that for (int i = 0; ... in C++ defines i in the outer scope not the loop scope, which is why I never use it. However, if the desire is really to poison the loop variable then we can do #define list_for_each_entry(pos, head, member) \ for (pos = list_first_entry(head, typeof(*pos), member); \ !list_entry_is_head(pos, head, member) && ((pos = NULL) == NULL; \ pos = list_next_entry(pos, member)) Which would at least set pos to NULL when the loop completes. > Besides, there are no guarantees that people won't > do_something_with(entry) without the check or won't compare entry to > NULL to check if the loop finished with break or not. I get the wider goal, but we have to patch the problem cases now and a simple one-liner is better than a larger patch that may or may not work if we ever achieve the local definition or value poisoning idea. I'm also fairly certain coccinelle can come up with a use without checking for loop completion semantic patch which we can add to 0day. James