Re: [RFC PATCH 04/13] vfio/mdev: remove the usage of the list iterator after the loop

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Jakob <jakobkoschel@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-arch <linux-arch@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Arnd Bergman <arnd@arndb.de>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Kees Cook <keescook@chromium.org>,
	Mike Rapoport <rppt@kernel.org>,
	"Gustavo A. R. Silva" <gustavo@embeddedor.com>,
	Brian Johannesmeyer <bjohannesmeyer@gmail.com>,
	Cristiano Giuffrida <c.giuffrida@vu.nl>,
	"Bos, H.J." <h.j.bos@vu.nl>
Subject: Re: [RFC PATCH 04/13] vfio/mdev: remove the usage of the list iterator after the loop
Date: Wed, 23 Feb 2022 11:31:10 -0800	[thread overview]
Message-ID: <CAHk-=widDQUjQS2tpaw3j_+Yz8rAY3P0qdqpz+nTNu4-3LaU3w@mail.gmail.com> (raw)
In-Reply-To: <20220223191222.GC10361@ziepe.ca>

On Wed, Feb 23, 2022 at 11:12 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> Yes, this is what I had put together as well about this patch, and I
> think it is OK as-is. In this case the list head is in the .bss of a
> module so I don't think it is very likely that the type confused
> container_of() will alias a kalloc result, but it is certainly
> technically wrong as-is.

I think that the pattern we should strive to use is not top use a
'bool' with the

 - initialize to false, and then in loop: do 'found = true;' if found

 - use the iterator variable if 'found'.

but instead strive to

 - either only use the iterator variable inside the loop

 - if you want to use it after the loop, have a externally visible
separate pointer initialized to NULL, and set it to the iterator
variable inside the loop

IOW, instead of having a variable that is a 'bool', just make that
variable _be_ the pointer you want. It's clearer, and it avoids using
the iterator variable outside the loop.

It also is likely to generate better code, because there are fewer
live variables - outside the loop now only uses that one variable,
rather than using the 'bool' variable _and_ the iterator variable.

               Linus