Hi Andrew,

thanks for the review.

Andrew Morton wrote:
matthieu castet <[EMAIL PROTECTED]> wrote:


...
+static int request_cmvs(struct uea_softc *sc,
+                struct uea_cmvs **cmvs, const struct firmware **fw)
+{
+       int ret, size;
+       u8 *data;
+       char *file;
+       char cmv_name[256] = FW_DIR;


That's rather a lot of stack.  Can this be made static, of kmalloced?


I think we'll made it static.

+
+       *cmvs = (struct uea_cmvs *)(data + 1);


That's a bit rude - asking the compiler to perform a structure copy from an
odd address.  memcpy() would be saner.

Could you elaborate a bit more ?
I don't see where there is a copy.
*cmvs is a pointer to the structure, not the structure. And when we parse the structure, we use get_unaligned functions.


...
+/**
+ * uea_read_proc : /proc information
+ */
+static int uea_read_proc(char *page, + char **start, off_t off, int count, int *eof, void *data)


People get shouted at for adding /proc handlers.  Greg may have thoughts...

Ok, we may be convert some values to sysfs. It would be nice if usbatm allow us to export some common value (margin, ...).


Matthieu


-------------------------------------------------------
This SF.Net email is sponsored by the JBoss Inc.
Get Certified Today * Register for a JBoss Training Course
Free Certification Exam for All Training Attendees Through End of 2005
Visit http://www.jboss.com/services/certification for more information
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to