linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Geert Uytterhoeven <geert@linux-m68k.org>
Subject: Re: [PATCH v1 1/1] drm/i915/selftests: Replace too verbose for-loop with simpler while
Date: Tue, 15 Feb 2022 19:14:49 +0200	[thread overview]
Message-ID: <87o83859x2.fsf@intel.com> (raw)
In-Reply-To: <20220215163213.54917-1-andriy.shevchenko@linux.intel.com>

On Tue, 15 Feb 2022, Andy Shevchenko <andriy.shevchenko@linux.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@linux.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

  reply	other threads:[~2022-02-15 17:15 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-15 16:32 [PATCH v1 1/1] drm/i915/selftests: Replace too verbose for-loop with simpler while Andy Shevchenko
2022-02-15 17:14 ` Jani Nikula [this message]
2022-02-15 18:58   ` Andy Shevchenko
2022-02-16  8:55     ` Jani Nikula
2022-02-16  9:02       ` Geert Uytterhoeven

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87o83859x2.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=airlied@linux.ie \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=geert@linux-m68k.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rodrigo.vivi@intel.com \
    --cc=tvrtko.ursulin@linux.intel.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).