Re: [PATCH] serial: 8250_omap: fix a timeout loop condition

Andy Shevchenko
2021-04-30T14:22:08Z

On Fri, Apr 30, 2021 at 04:33:29PM +0300, Dan Carpenter wrote:
> On Fri, Apr 30, 2021 at 03:53:44PM +0300, Andy Shevchenko wrote:
> > On Fri, Apr 30, 2021 at 02:41:06PM +0300, Dan Carpenter wrote:
> > > On Fri, Apr 30, 2021 at 11:46:07AM +0300, Andy Shevchenko wrote:

...

> > > Why would I make it unsigned? As a static analysis developer,
> > > pointlessly unsigned variables are one of the leading causes for the
> > > bugs I see.
> > >
> > > There are times where a iterator counter needs to be unsigned long, or
> > > u64 but I have never seen a case where changing an iterator from
> > > "int i;" to "unsigned int i;" solves a real life kernel bug. It only
> > > introduces bugs.
> >
> > See my followup to that, I meant
> >
> > unsigned int count;
> >
> > do {
> > ...
> > } while (--count);
> >
> > It doesn't solve bug, but prevents the code be read incorrectly like what you
> > are fixing can be avoided with do {} while (); along with unsigned type.
>
> Why would you use an unsigned int for this???

Why it should be signed? You clearly show the amount of iterations. Check for
null I guess even compact in the assembly in comparison to -1.

I do not see any point why it should be signed. For what purpose?

It's a *down* counter.

--
With Best Regards,
Andy Shevchenko