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 DCB38C433EF for ; Tue, 1 Mar 2022 23:56:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238702AbiCAX44 (ORCPT ); Tue, 1 Mar 2022 18:56:56 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41860 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235135AbiCAX4z (ORCPT ); Tue, 1 Mar 2022 18:56:55 -0500 Received: from mail-ej1-x62f.google.com (mail-ej1-x62f.google.com [IPv6:2a00:1450:4864:20::62f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 526B670336 for ; Tue, 1 Mar 2022 15:56:13 -0800 (PST) Received: by mail-ej1-x62f.google.com with SMTP id qx21so162007ejb.13 for ; Tue, 01 Mar 2022 15:56:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=4H0hy1V+vsSL/J4iRHU98fs6Ghx9E/bawvPg5OjXQmQ=; b=A1xSV6w+mnGI8KddmZFT+SVp47UhRqZlj7dZE7Ue1BgWjGw2+QCc51Zb6PIPvKiXuh /TGd1P3o63R0kiFyLwVU+kzDBf6Kt9wFSZwwJYnrWAqavu9szp3nfmKm0Qe68N3l8Mdl bWvesgtwBf4BWRSm4aUiCmj0pmAaHYwmzAjUs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=4H0hy1V+vsSL/J4iRHU98fs6Ghx9E/bawvPg5OjXQmQ=; b=Fl0VC3IJ/QceLCa8Wu/q4vkh+se38+9DVNDDP55zsrB0sfTW7mcXe7fVIOMAPUBekl 7Ol6FtP3yV94Q8Ts48/IEsxc6VtPXK93zHHR7DByRthitpRe4tYDiinxVezg5SeE1YM7 5pfHEExNjcZ+XhYzzDj5VG7C9j3bUeRg2bGDIOJ4wL7DWjfXqoNR7j2skhYYW/qe5BV0 2geAPbxw+T8HZ3j9sh1HzRCCRvEAmY5TU95avZeA0CUCq1LzCcHjcEm/m2ozvTsgGrEt Dh9e/YGK9762YoDKmdn0oYpEdIPJeP/x9eEy5/Ft3+NfLbR5gtMtWrnJuL+g+4W2Bs0C LNHQ== X-Gm-Message-State: AOAM530EYbf21LMQrlbudYS/QDM/vOzftEtAFP/1FER3Vp+4eG4XyBgt ztK4WGeqNI8wem+98oLOqrnL1t0gHX8agJq5elk= X-Google-Smtp-Source: ABdhPJxY1lgejHNcJWGmWt2k0l4UPhVbJLG8lfBLDctaLgJjlwx0181aKNgMoj522OMRwXa8zrjOYA== X-Received: by 2002:a17:906:53c7:b0:6ce:6f32:ce53 with SMTP id p7-20020a17090653c700b006ce6f32ce53mr20890403ejo.352.1646178971667; Tue, 01 Mar 2022 15:56:11 -0800 (PST) Received: from mail-ej1-f44.google.com (mail-ej1-f44.google.com. [209.85.218.44]) by smtp.gmail.com with ESMTPSA id hc43-20020a17090716ab00b006c7622abe73sm5887580ejc.216.2022.03.01.15.56.09 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 01 Mar 2022 15:56:10 -0800 (PST) Received: by mail-ej1-f44.google.com with SMTP id p15so214019ejc.7 for ; Tue, 01 Mar 2022 15:56:09 -0800 (PST) X-Received: by 2002:a05:6512:3042:b0:437:96f5:e68a with SMTP id b2-20020a056512304200b0043796f5e68amr17643498lfb.449.1646178958685; Tue, 01 Mar 2022 15:55:58 -0800 (PST) MIME-Version: 1.0 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> <73fa82a20910c06784be2352a655acc59e9942ea.camel@HansenPartnership.com> <7dc860874d434d2288f36730d8ea3312@AcuMS.aculab.com> <0ced2b155b984882b39e895f0211037c@AcuMS.aculab.com> In-Reply-To: <0ced2b155b984882b39e895f0211037c@AcuMS.aculab.com> From: Linus Torvalds Date: Tue, 1 Mar 2022 15:55:42 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr To: David Laight Cc: James Bottomley , linux-wireless , "alsa-devel@alsa-project.org" , KVM list , "Gustavo A. R. Silva" , "linux-iio@vger.kernel.org" , "nouveau@lists.freedesktop.org" , Rasmus Villemoes , dri-devel , Cristiano Giuffrida , "Bos, H.J." , "linux1394-devel@lists.sourceforge.net" , "drbd-dev@lists.linbit.com" , linux-arch , CIFS , "linux-aspeed@lists.ozlabs.org" , linux-scsi , linux-rdma , "linux-staging@lists.linux.dev" , amd-gfx list , 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 , dma , Christophe JAILLET , Jakob Koschel , "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" , "samba-technical@lists.samba.org" , Linux Kernel Mailing List , Linux F2FS Dev Mailing List , "tipc-discussion@lists.sourceforge.net" , Linux Crypto Mailing List , linux-fsdevel , "linux-mediatek@lists.infradead.org" , Andrew Morton , linuxppc-dev , =?UTF-8?Q?Christian_K=C3=B6nig?= , Mike Rapoport Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Tue, Mar 1, 2022 at 3:19 PM David Laight wrote: > > Having said that there are so few users of list_entry_is_head() > it is reasonable to generate two new names. Well, the problem is that the users of list_entry_is_head() may be few - but there are a number of _other_ ways to check "was that the HEAD pointer", and not all of them are necessarily correct. IOW, different places do different random tests for "did we walk the whole loop without breaking out". And many of them happen to work. In fact, in practice, pretty much *all* of them happen to work, and you have to have the right struct layout and really really bad luck to hit a case of "type confusion ended up causing the test to not work". And *THAT* is the problem here. It's not the "there are 25ish places that current use list_entry_is_head()". It's the "there are ~480 places that use the type-confused HEAD entry that has been cast to the wrong type". And THAT is why I think we'd be better off with that bigger change that simply means that you can't use the iterator variable at all outside the loop, and try to make it something where the compiler can help catch mis-uses. Now, making the list_for_each_entry() thing force the iterator to NULL at the end of the loop does fix the problem. The issue I have with it is really just that you end up getting no warning at all from the compiler if you mix old-style and new-style semantics. Now, you *will* get an oops (if using a new-style iterator with an old-style check), but many of these things will be in odd driver code and may happen only for error cases. And if you use a new-style check with an old-style iterator (ie some backport problem), you will probably end up getting random memory corruption, because you'll decide "it's not a HEAD entry", and then you'll actually *use* the HEAD that has the wrong type cast associated with it. See what my worry is? With the "don't use iterator outside the loop" approach, the exact same code works in both the old world order and the new world order, and you don't have the semantic confusion. And *if* you try to use the iterator outside the loop, you'll _mostly_ (*) get a compiler warning about it not being initialized. Linus (*) Unless somebody initializes the iterator pointer pointlessly. Which clearly does happen. Thus the "mostly". It's not perfect, and that's most definitely not nice - but it should at least hopefully make it that much harder to mess up.