On Wed, Nov 17, 2010 at 12:50:10PM -0700, Pete Zaitcev wrote:
> On Wed, 17 Nov 2010 11:40:22 -0800
> Matthew Dharm <mdharm-usb@xxxxxxxxxxxxxxxxxx> wrote:
>
> > > > > + length = 0;
> > > > > + fp = urb->iso_frame_desc;
> > > > > + while (ndesc-- != 0) {
> > > > > + if (fp->actual_length != 0) {
> > > > > + if (fp->offset + fp->actual_length > length)
> > > > > + length = fp->offset + fp->actual_length;
> > > > > + }
> > > >
> > > > {} are not needed here, and thwo *if* statements can be collapsed into one.
> > >
> > > They are semantically different and thus they were left uncollaplsed.
> > > The upper one selects the descriptor and the lower own is an
> > > open-coded min. Also, any collapsing would lead to hideously ugly
> > > continuation lines.
> >
> > What about doing the internal if() with a MIN() instead? It looks
> > equivalent to
> > length = MIN(length, fp->offset + fp->actual_length);
>
> That looks good, I just keep forgetting what min and MIN do,
> and the typing restrictions... Need to read up on them.
Do you want to do a new patch here, or just send a fix-up to go on top
of this one? Either is fine with me.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
