On 27.03.2018 09:36, Geert Uytterhoeven wrote:
> Hi Andrzej,
>
> 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" ? :)
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.
I guess there were somewhere discussion about it, could you point me to
it if possible, to avoid unnecessary noise in this thread.
Regards
Andrzej
>
> Gr{oetje,eeting}s,
>
> Geert
>
--
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
