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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux