linux-kernel - Re: [PATCH v1 1/1] drm/i915/selftests: Replace too verbose for-loop with simpler while

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 15 Feb 2022 19:14:49 +0200
From:   Jani Nikula <jani.nikula@...ux.intel.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        intel-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org
Cc:     Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Geert Uytterhoeven <geert@...ux-m68k.org>
Subject: Re: [PATCH v1 1/1] drm/i915/selftests: Replace too verbose for-loop
 with simpler while

On Tue, 15 Feb 2022, Andy Shevchenko <andriy.shevchenko@...ux.intel.com> wrote:
> It's hard to parse for-loop which has some magic calculations inside.
> Much cleaner to use while-loop directly.

I assume you're trying to prove a point following our recent
for-vs-while loop discussion. I really can't think of any other reason
you'd end up looking at this file or this loop.

With the change, the loop indeed becomes simpler, but it also runs one
iteration further than the original. Whoops.

It's a selftest. The loop's been there for five years. What are we
trying to achieve here? So we disagree on loops, fine. Perhaps this is
not the best use of either of our time? Please just let the for loops in
i915 be.


BR,
Jani.

>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> ---
>  drivers/gpu/drm/i915/selftests/i915_syncmap.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_syncmap.c b/drivers/gpu/drm/i915/selftests/i915_syncmap.c
> index 47f4ae18a1ef..26981d1c3025 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_syncmap.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_syncmap.c
> @@ -36,10 +36,10 @@ __sync_print(struct i915_syncmap *p,
>  	unsigned int i, X;
>  
>  	if (depth) {
> -		unsigned int d;
> +		unsigned int d = depth;
>  
> -		for (d = 0; d < depth - 1; d++) {
> -			if (last & BIT(depth - d - 1))
> +		while (d--) {
> +			if (last & BIT(d))
>  				len = scnprintf(buf, *sz, "|   ");
>  			else
>  				len = scnprintf(buf, *sz, "    ");

-- 
Jani Nikula, Intel Open Source Graphics Center