Hi Andrezj,
On Tue, Mar 27, 2018 at 10:36 AM, Andrzej Hajda <a.hajda@xxxxxxxxxxx> wrote:
> On 27.03.2018 09:36, Geert Uytterhoeven wrote:
>> On Tue, Mar 27, 2018 at 9:28 AM, Andrzej Hajda <a.hajda@xxxxxxxxxxx> 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.
>
> Wow, it sounds crazy, shall we expect gigantic patchsets, converting all
> occurrences of int to "unsigned int" ? :)
No we shall not.
> I know both types have their pros and cons and can behave unexpectedly
> in corner cases, but I do not see why unsigned should be preferred over
> signed in general, or in this particular case.
When looping over array indices, and comparing with ARRAY_SIZE (which
is unsigned), using "unsigned int" is preferred.
However, in this case the error code relies on the index becoming negative,
so a signed integer should be used.
> I guess there were somewhere discussion about it, could you point me to
> it if possible, to avoid unnecessary noise in this thread.
Not here, but Google pointed me to
http://blog.robertelder.org/signed-or-unsigned/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
