Re: [PATCH] 2.5.59 : sound/oss/vidc.c

From: Richard B. Johnson (root@chaos.analogic.com)
Date: Fri Feb 07 2003 - 15:47:28 EST


On Fri, 7 Feb 2003, John Bradford wrote:

> > > --- linux/sound/oss/vidc.c.old 2003-01-16 21:21:38.000000000 -0500
> > > +++ linux/sound/oss/vidc.c 2003-02-07 02:59:44.000000000 -0500
> > > @@ -225,7 +225,7 @@
> > > newsize = 208;
> > > if (newsize > 4096)
> > > newsize = 4096;
> > > - for (new2size = 128; new2size < newsize; new2size <<= 1);
> > > + for (new2size = 128; new2size < newsize; new2size <<= 1)
> > > if (new2size - newsize > newsize - (new2size >> 1))
> > > new2size >>= 1;
> > > if (new2size > 4096) {
> >
> > The code is correct as it originally stood.
> >
> > It looks like indent has a bug and incorrectly formats this to look wrong.
> > Back in 2.2 times, the code looks like this:
> >
> > for (new2size = 128; new2size < newsize; new2size <<= 1);
> > if (new2size - newsize > newsize - (new2size >> 1))
> > new2size >>= 1;
> >
> > and this is the intended functionality. Please do NOT apply the patch.
>
> I thought we were switching to negative tab indentation, anyway:
>
> http://marc.theaimsgroup.com/?l=linux-kernel&m=104125431009832&w=2
>
> :-)
>
> John.

If the code is correct, it really should be written as:

   for (new2size = 128; new2size < newsize; new2size <<= 1)
       ;

The code seems to want to make the value of new2size a power of
2 and, greater than 128, but less than newsize. It ignores the
fact that newsize might be less than 128, maybe this is okay.
But, the code goes on, eventually settling on new2size being
less than 4096... hmmm. I'll bet this could be greatly
simplified. I think 'new2size' is really something that will
fit inside 128-byte groups. Maybe an & or a % will greatly
simplify?

Cheers,
Dick Johnson
Penguin : Linux version 2.4.18 on an i686 machine (797.90 BogoMips).
Why is the government concerned about the lunatic fringe? Think about it.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Fri Feb 07 2003 - 22:00:24 EST