linux-kernel - Re: [PATCH v4 1/6] drm/format-helper: Add drm_fb_xrgb8888_to_gray8

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]

Message-ID: <CAMuHMdWJ_T5HoWcOuSCDP-uquqpiWGPUmKNaC9U5d=JkGRGP0g@mail.gmail.com>
Date:   Fri, 11 Feb 2022 13:27:29 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Jani Nikula <jani.nikula@...ux.intel.com>
Cc:     Thomas Zimmermann <tzimmermann@...e.de>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Javier Martinez Canillas <javierm@...hat.com>,
        Linux Fbdev development list <linux-fbdev@...r.kernel.org>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        Noralf Trønnes <noralf@...nnes.org>,
        Maxime Ripard <maxime@...no.tech>,
        Sam Ravnborg <sam@...nborg.org>
Subject: Re: [PATCH v4 1/6] drm/format-helper: Add drm_fb_xrgb8888_to_gray8_line()

Hi Jani,

On Fri, Feb 11, 2022 at 1:06 PM Jani Nikula <jani.nikula@...ux.intel.com> wrote:
> On Fri, 11 Feb 2022, Thomas Zimmermann <tzimmermann@...e.de> wrote:
> > Am 11.02.22 um 12:12 schrieb Andy Shevchenko:
> >> On Fri, Feb 11, 2022 at 11:40:13AM +0100, Javier Martinez Canillas wrote:
> >>> On 2/11/22 11:28, Andy Shevchenko wrote:
> >>>> On Fri, Feb 11, 2022 at 10:19:22AM +0100, Javier Martinez Canillas wrote:
> >>
> >> ...
> >>
> >>>>> +static void drm_fb_xrgb8888_to_gray8_line(u8 *dst, const u32 *src, unsigned int pixels)
> >>>>> +{
> >>>>> + unsigned int x;
> >>>>> +
> >>>>> + for (x = 0; x < pixels; x++) {
> >>>>> +         u8 r = (*src & 0x00ff0000) >> 16;
> >>>>> +         u8 g = (*src & 0x0000ff00) >> 8;
> >>>>> +         u8 b =  *src & 0x000000ff;
> >>>>> +
> >>>>> +         /* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */
> >>>>> +         *dst++ = (3 * r + 6 * g + b) / 10;
> >>>>> +         src++;
> >>>>> + }
> >>>>
> >>>> Can be done as
> >>>>
> >>>>    while (pixels--) {
> >>>>            ...
> >>>>    }
> >>>>
> >>>> or
> >>>>
> >>>>    do {
> >>>>            ...
> >>>>    } while (--pixels);
> >>>>
> >>>
> >>> I don't see why a while loop would be an improvement here TBH.
> >>
> >> Less letters to parse when reading the code.
> >
> > It's a simple refactoring of code that has worked well so far. Let's
> > leave it as-is for now.
>
> IMO *always* prefer a for loop over while or do-while.

(guess what ;-) I tend to disagree.

> The for (i = 0; i < N; i++) is such a strong paradigm in C. You
> instantly know how many times you're going to loop, at a glance. Not so
> with with the alternatives, which should be used sparingly.

In this case it's fairly obvious, and you get rid of the extra variable x.
Less code, less variables, what can go wrong? ;-)

> And yes, the do-while suggested above is buggy, and you actually need to
> stop and think to see why.

Yes, that one is wrong.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds