On Tue, 30 Jun 2009 09:13:23 +0200 Oliver Neukum <oliver@xxxxxxxxxx> wrote:
> Am Dienstag, 30. Juni 2009 02:08:16 schrieb akpm@xxxxxxxxxxxxxxxxxxxx:
>
> > @@ -1658,7 +1658,6 @@ static int ti_do_download(struct usb_dev
> > u8 cs = 0;
> > int done;
> > struct ti_firmware_header *header;
> > - int status;
> > int len;
> >
> > for (pos = sizeof(struct ti_firmware_header); pos < size; pos++)
> > @@ -1671,13 +1670,15 @@ static int ti_do_download(struct usb_dev
> >
> > dbg("%s - downloading firmware", __func__);
> > for (pos = 0; pos < size; pos += done) {
> > + int status;
> > +
> > len = min(size - pos, TI_DOWNLOAD_MAX_PACKET_SIZE);
> > status = usb_bulk_msg(dev, pipe, buffer + pos, len,
> > &done, 1000);
> > if (status)
> > - break;
> > + return status;
>
> This kind of fix doesn't seem to be a good idea. You always run the risk
> that somebody wants to add error handling or locking and will have problems.
> Can't you just initialize the variable and be done with it instead of this ugly
> per block which always causes potential problems with overshadowing?
>
I knew there would be something wrong with it.
Please treat the patch as a bug report and get it fixed asap?
--
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
