On Thu, Jan 14, 2010 at 07:08:38PM +0100, Stefan Brüns wrote:
> On Friday 08 January 2010 20:56:16 you wrote:
> > On Fri, Jan 08, 2010 at 08:07:51PM +0100, Stefan Brüns wrote:
> > > get rid of temporary iso frame descriptor (isopkt)
> >
> > Why? I don't think you can rely on access_ok, copy_from_user() is
> > "safer" from what I remember.
>
> You are right on this one, copy_{from,to}_user takes paging into account.
>
> > > + if(uurb->type != USBDEVFS_URB_TYPE_ISO) {
> > > + as = alloc_async(0);
> > > + if (!as)
> > > + return -ENOMEM;
> > > + }
> >
> > Why allocate one if we aren't going to use it?
>
> Hm, dont understand this comment. /as/ will be used, except for error cases.
>
> > Also, can you run your patch through 'scripts/checkpatch.pl' to catch
> > the minor formatting issues?
>
> done
>
> Stefan
>
> --
> Stefan Brüns / Bergstraße 21 / 52062 Aachen
> phone: +49 241 53809034 mobile: +49 151 50412019
> From edee1b56def24fa4142b789320d3e0578c4653b5 Mon Sep 17 00:00:00 2001
> From: =?utf-8?q?Stefan=20Br=C3=BCns?= <stefan.bruens@xxxxxxxxxxxxxx>
> Date: Fri, 8 Jan 2010 19:49:08 +0100
> Subject: [PATCH] usbdevfs: allocate async early, to allow some cleanup
> MIME-Version: 1.0
> Content-Type: text/plain; charset=utf-8
> Content-Transfer-Encoding: 8bit
>
> - only set urb fields if applicable for respective urb type
> - traverse iso pkt descriptor only once, free it immediately when
> it is no longer needed.
I'm missing something, what problems does this patch fix?
Maybe if you broke it up into 2 different patches (remember a patch
should only do one thing), it might be easier to understand.
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
