linux-kernel - Re: [PATCH v6 2/3] drm: bridge: Add thc63lvd1024 LVDS decoder driver

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, 27 Mar 2018 09:36:15 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Andrzej Hajda <a.hajda@...sung.com>
Cc:     Vladimir Zapolskiy <vladimir_zapolskiy@...tor.com>,
        Jacopo Mondi <jacopo+renesas@...ndi.org>,
        Archit Taneja <architt@...eaurora.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        David Airlie <airlied@...ux.ie>,
        Simon Horman <horms@...ge.net.au>,
        Magnus Damm <magnus.damm@...il.com>,
        Niklas Söderlund <niklas.soderlund@...natech.se>,
        Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6 2/3] drm: bridge: Add thc63lvd1024 LVDS decoder driver

Hi Andrzej,

On Tue, Mar 27, 2018 at 9:28 AM, Andrzej Hajda <a.hajda@...sung.com> wrote:
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c

>>> +static void thc63_enable(struct drm_bridge *bridge)
>>> +{
>>> +    struct thc63_dev *thc63 = to_thc63(bridge);
>>> +    struct regulator *vcc;
>>> +    int i;
>> unsigned int i;
>
> Why? You are introducing silly bug this way, see few lines below.
>
>>
>>> +
>>> +    for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++) {
>>> +            vcc = thc63->vccs[i];
>>> +            if (!vcc)
>>> +                    continue;
>>> +
>>> +            if (regulator_enable(vcc))
>>> +                    goto error_vcc_enable;
>>> +    }
>>> +
>>> +    if (thc63->pdwn)
>>> +            gpiod_set_value(thc63->pdwn, 0);
>>> +
>>> +    if (thc63->oe)
>>> +            gpiod_set_value(thc63->oe, 1);
>>> +
>>> +    return;
>>> +
>>> +error_vcc_enable:
>>> +    dev_err(thc63->dev, "Failed to enable regulator %s\n",
>>> +            thc63_reg_names[i]);
>>> +
>>> +    for (i = i - 1; i >= 0; i--) {
>
> Here, the loop will not end if you define i unsigned.

True.

> I know one can change the loop, to make it working with unsigned. But
> this clearly shows that using unsigned is more risky.
> What are advantages of unsigned vs int in this case. Are there some
> guidelines/discussions about it?

Some people consider signed integers harmful, as they may be converted
silently by the compiler to the "larger" unsigned type when needed.

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