From: Jani Nikula <jani.nikula@linux.intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>,
"Thomas Zimmermann" <tzimmermann@suse.de>
Cc: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 8/8] drm/client: s/unsigned int i/int i/
Date: Wed, 09 Oct 2024 17:32:52 +0300 [thread overview]
Message-ID: <87r08p8hyj.fsf@intel.com> (raw)
In-Reply-To: <ZwWED3yDPKfMsNPA@intel.com>
On Tue, 08 Oct 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Mon, Oct 07, 2024 at 09:43:47AM +0200, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 03.10.24 um 13:33 schrieb Ville Syrjala:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > Replace the 'unsigned int i' footguns with plain old signed
>> > int. Avoids accidents if/when someone decides they need
>> > to iterate backwards.
>>
>> Why are signed types preferable here?
>
> If you iterate backwards you typically write
>
> for (i = max; i >= 0; i--) {...}
>
> and i>=0 is always true for unsigned types.
>
> Another danger is doing any kind of arithmetic
> with 'i' and expecting a signed result.
>
> Based on my experience in getting burned by C integer
> promotion/converison rules a good rule of thumb is to
> always use just "int" unless there is a very good
> reason for not doing so (eg. if the thing is a bitmask
> or some kind of other thing where negative values
> can never ever come up).
Agreed.
An even worse antipattern is using u8 or u16 just because it's the
smallest type that is enough for the range or whatever. But then it ends
up being signed int arithmetic assigned back to the small unsigned type
anyway.
> Also IIRC there was a Linus rant about "unsigned int i"
> but I can't find it now.
Another summary at [1].
BR,
Jani.
[1] https://hamstergene.github.io/posts/2021-10-30-do-not-use-unsigned-for-nonnegativity/
--
Jani Nikula, Intel
next prev parent reply other threads:[~2024-10-09 14:33 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-03 11:32 [PATCH 0/8] drm/client: Stop using legacy crtc->mode and a bunch of cleanups Ville Syrjala
2024-10-03 11:32 ` [PATCH 1/8] drm/client: Constify modes Ville Syrjala
2024-10-04 23:59 ` kernel test robot
2024-10-05 0:19 ` kernel test robot
2024-10-03 11:32 ` [PATCH 2/8] drm/client: Use array notation for function arguments Ville Syrjala
2024-10-03 11:32 ` [PATCH 3/8] drm/client: Streamline mode selection debugs Ville Syrjala
2024-10-03 11:33 ` [PATCH 4/8] drm/client: Make copies of modes Ville Syrjala
2024-10-03 16:45 ` Ville Syrjälä
2024-10-03 18:15 ` [PATCH v2 " Ville Syrjala
2024-10-07 7:36 ` [PATCH " Thomas Zimmermann
2024-10-08 19:33 ` Ville Syrjälä
2024-10-10 19:28 ` Ville Syrjälä
2024-10-09 14:09 ` kernel test robot
2024-10-03 11:33 ` [PATCH 5/8] drm/client: Stop using the legacy crtc->mode Ville Syrjala
2024-10-03 18:16 ` [PATCH v2 " Ville Syrjala
2024-10-03 11:33 ` [PATCH 6/8] drm/client: s/new_crtc/crtc/ Ville Syrjala
2024-10-03 18:17 ` [PATCH v2 " Ville Syrjala
2024-10-03 11:33 ` [PATCH 7/8] drm/client: Move variables to tighter scope Ville Syrjala
2024-10-03 14:59 ` Ville Syrjälä
2024-10-03 11:33 ` [PATCH 8/8] drm/client: s/unsigned int i/int i/ Ville Syrjala
2024-10-07 7:43 ` Thomas Zimmermann
2024-10-08 19:12 ` Ville Syrjälä
2024-10-09 14:32 ` Jani Nikula [this message]
2024-10-03 17:57 ` ✗ Fi.CI.CHECKPATCH: warning for drm/client: Stop using legacy crtc->mode and a bunch of cleanups Patchwork
2024-10-03 18:07 ` ✗ Fi.CI.BAT: failure " Patchwork
2024-10-03 21:40 ` ✗ Fi.CI.CHECKPATCH: warning for drm/client: Stop using legacy crtc->mode and a bunch of cleanups (rev4) Patchwork
2024-10-03 21:49 ` ✓ Fi.CI.BAT: success " Patchwork
2024-10-08 6:36 ` ✓ Fi.CI.IGT: " Patchwork
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=87r08p8hyj.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=tzimmermann@suse.de \
--cc=ville.syrjala@linux.intel.com \
/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