Combine buffer copy and usb string creation into a single operation.

Also, struct cp210x_buffer need not be packed.

diff --git a/src/cp210x.c b/src/cp210x.c
index af0302c1ba61613fd0563427ce4f47c1e659b70f..b7de40f900f28a2c21e8cd526b7de2d1f805eae3 100644 (file)
--- a/src/cp210x.c
+++ b/src/cp210x.c
@@ -207,25 +207,64 @@ static struct usb_serial_driver cp210x_device = {
 #define CP210x_PART_CP2102     0x02
 #define CP210x_PART_CP2103     0x03
 
+/* Helper to make usb string size */
+#define USBSTRLEN(x)           (x * 2 + 2)
+
+/* Populates usbstr with: (len) + (0x03) + unicode(str).  Each char in str
+ * takes two bytes in unicode format.
+ * Returns the resulting length of the string in usbstr.
+ * This function can accept overlapping usbstr and str as long as the overlap
+ * does not cause data written to usbstr to overwrite data not yet read from
+ * str.
+ */
+static int make_usb_string(char *usbstr, size_t usblen, char *src,
+               size_t srclen)
+{
+       int len = 0;
+
+       if (usbstr && usblen >= 2 && src && *src && srclen) {
+               char *p;
+
+               if (usblen > 255)
+                       usblen = 255;
+
+               p = usbstr + 1;
+               *p++ = 0x03;
+               len = 2;
+               while (srclen && len < usblen) {
+                       *p++ = *src++;
+                       *p++ = 0;
+                       len += 2;
+                       srclen--;
+               }
+               *usbstr = (char)len;
+       }
+       return len;
+}
+
 /*
- * cp210x_buf_from_user
- * Copy a buffer from user space, returning the number of bytes copied
- * from ubuf.buf into kbuf.  klen is the size of the buffer at kbuf.
+ * cp210x_usbstr_from_user
+ * Populate kbuf with a usb string derived from a user space variable.  klen
+ * is the size of the buffer at kbuf.
+ * Returns the number of bytes used in kbuf.
  */
-static size_t cp210x_buf_from_user(char *kbuf, unsigned long ubuf, size_t klen)
+static size_t cp210x_usbstr_from_user(char *kbuf, unsigned long ubuf, size_t klen)
 {
        struct cp210x_buffer t;
+       char* str;
 
-       if (!kbuf || !ubuf || !klen ||
-                       copy_from_user(&t, (struct cp210x_buffer __user *)ubuf,
-                       sizeof(t)))
+       if (!kbuf || !ubuf || !klen)
+               return 0;
+       if (copy_from_user(&t, (struct cp210x_buffer __user *)ubuf, sizeof(t)))
+               return 0;
+       if (!t.buf || !t.len)
                return 0;
        if (t.len < klen)
                klen = t.len;
-       if (!t.buf || !t.len ||
-                       copy_from_user(kbuf, (u8 __user *)t.buf, klen))
+       str = kbuf + klen - t.len;
+       if (copy_from_user(kbuf + klen - t.len, (u8 __user *)t.buf, klen))
                return 0;
-       return klen;
+       return make_usb_string(kbuf, klen, str, t.len);
 }
 
 /* cp210x_has_setmfg
@@ -319,39 +358,6 @@ static inline int cp210x_setu16(struct usb_serial_port *port, int cmd,
                        value, NULL, 0);
 }
 
-/* Populates usbstr with: (len) + (0x03) + unicode(str).  Each char in str
- * takes up two bytes in unicode format, so the resulting len(usbstr) is
- * 2 * len(str) + 2.
- * Returns the resulting length of the string in usbstr.
- * This function can accept overlapping usbstr and str as long as the overlap
- * does not cause data written to usbstr to overwrite data not yet read from
- * str.
- */
-static int make_usb_string(char *usbstr, size_t usblen, char *src,
-               size_t srclen)
-{
-       int len = 0;
-
-       if (usbstr && usblen >= 2 && src && *src && srclen) {
-               char *p;
-
-               if (usblen > 255)
-                       usblen = 255;
-
-               p = usbstr + 1;
-               *p++ = 0x03;
-               len = 2;
-               while (srclen && len < usblen) {
-                       *p++ = *src++;
-                       *p++ = 0;
-                       len += 2;
-                       srclen--;
-               }
-               *usbstr = (char)len;
-       }
-       return len;
-}
-
 /*
  * cp210x_setstr
  *
@@ -765,11 +771,9 @@ static int cp210x_ioctl(struct usb_serial_port *port, struct file *file,
 
        case IOCTL_SETMFG:
        if (cp210x_has_setmfg()) {
-               char usbstr[CP210x_MAX_MFG_STRLEN * 2 + 2];
-               char *str = usbstr + sizeof(usbstr) - CP210x_MAX_MFG_STRLEN;
-               size_t len = cp210x_buf_from_user(str, arg,
-                               CP210x_MAX_MFG_STRLEN);
-               len = make_usb_string(usbstr, sizeof(usbstr), str, len);
+               char usbstr[USBSTRLEN(CP210x_MAX_MFG_STRLEN)];
+               size_t len = cp210x_usbstr_from_user(usbstr, arg,
+                               sizeof(usbstr));
                if (len && cp210x_setstr(port, 0x00, usbstr) == len)
                        return 0;
        }
@@ -778,11 +782,9 @@ static int cp210x_ioctl(struct usb_serial_port *port, struct file *file,
 
        case IOCTL_SETPRODUCT:
        {
-               char usbstr[CP210x_MAX_PRODUCT_STRLEN * 2 + 2];
-               char *str = usbstr + sizeof(usbstr) - CP210x_MAX_PRODUCT_STRLEN;
-               size_t len = cp210x_buf_from_user(str, arg,
-                               CP210x_MAX_PRODUCT_STRLEN);
-               len = make_usb_string(usbstr, sizeof(usbstr), str, len);
+               char usbstr[USBSTRLEN(CP210x_MAX_PRODUCT_STRLEN)];
+               size_t len = cp210x_usbstr_from_user(usbstr, arg,
+                               sizeof(usbstr));
                if (len && cp210x_setstr(port, 0x03, usbstr) == len)
                        return 0;
                return -EFAULT;
@@ -791,11 +793,9 @@ static int cp210x_ioctl(struct usb_serial_port *port, struct file *file,
 
        case IOCTL_SETSERIAL:
        {
-               char usbstr[CP210x_MAX_SERIAL_STRLEN * 2 + 2];
-               char *str = usbstr + sizeof(usbstr) - CP210x_MAX_SERIAL_STRLEN;
-               size_t len = cp210x_buf_from_user(str, arg,
-                               CP210x_MAX_SERIAL_STRLEN);
-               len = make_usb_string(usbstr, sizeof(usbstr), str, len);
+               char usbstr[USBSTRLEN(CP210x_MAX_SERIAL_STRLEN)];
+               size_t len = cp210x_usbstr_from_user(usbstr, arg,
+                               sizeof(usbstr));
                if (len && cp210x_setstr(port, 0x04, usbstr) == len)
                        return 0;
                return -EFAULT;

diff --git a/src/cp210x.c.karmic b/src/cp210x.c.karmic
index 78d6b62c1e0e1a84c0bae56825dd909ad6766224..6188b8a47d01f794794ac35cdd8c35f7f4546a48 100644 (file)
--- a/src/cp210x.c.karmic
+++ b/src/cp210x.c.karmic
@@ -228,25 +228,64 @@ static struct usb_serial_driver cp210x_device = {
 #define CP210x_PART_CP2102     0x02
 #define CP210x_PART_CP2103     0x03
 
+/* Helper to make usb string size */
+#define USBSTRLEN(x)           (x * 2 + 2)
+
+/* Populates usbstr with: (len) + (0x03) + unicode(str).  Each char in str
+ * takes two bytes in unicode format.
+ * Returns the resulting length of the string in usbstr.
+ * This function can accept overlapping usbstr and str as long as the overlap
+ * does not cause data written to usbstr to overwrite data not yet read from
+ * str.
+ */
+static int make_usb_string(char *usbstr, size_t usblen, char *src,
+               size_t srclen)
+{
+       int len = 0;
+
+       if (usbstr && usblen >= 2 && src && *src && srclen) {
+               char *p;
+
+               if (usblen > 255)
+                       usblen = 255;
+
+               p = usbstr + 1;
+               *p++ = 0x03;
+               len = 2;
+               while (srclen && len < usblen) {
+                       *p++ = *src++;
+                       *p++ = 0;
+                       len += 2;
+                       srclen--;
+               }
+               *usbstr = (char)len;
+       }
+       return len;
+}
+
 /*
- * cp210x_buf_from_user
- * Copy a buffer from user space, returning the number of bytes copied
- * from ubuf.buf into kbuf.  klen is the size of the buffer at kbuf.
+ * cp210x_usbstr_from_user
+ * Populate kbuf with a usb string derived from a user space variable.  klen
+ * is the size of the buffer at kbuf.
+ * Returns the number of bytes used in kbuf.
  */
-static size_t cp210x_buf_from_user(char *kbuf, unsigned long ubuf, size_t klen)
+static size_t cp210x_usbstr_from_user(char *kbuf, unsigned long ubuf, size_t klen)
 {
        struct cp210x_buffer t;
+       char* str;
 
-       if (!kbuf || !ubuf || !klen ||
-                       copy_from_user(&t, (struct cp210x_buffer __user *)ubuf,
-                       sizeof(t)))
+       if (!kbuf || !ubuf || !klen)
+               return 0;
+       if (copy_from_user(&t, (struct cp210x_buffer __user *)ubuf, sizeof(t)))
+               return 0;
+       if (!t.buf || !t.len)
                return 0;
        if (t.len < klen)
                klen = t.len;
-       if (!t.buf || !t.len ||
-                       copy_from_user(kbuf, (u8 __user *)t.buf, klen))
+       str = kbuf + klen - t.len;
+       if (copy_from_user(kbuf + klen - t.len, (u8 __user *)t.buf, klen))
                return 0;
-       return klen;
+       return make_usb_string(kbuf, klen, str, t.len);
 }
 
 /* cp210x_has_setmfg
@@ -340,39 +379,6 @@ static inline int cp210x_setu16(struct usb_serial_port *port, int cmd,
                        value, NULL, 0);
 }
 
-/* Populates usbstr with: (len) + (0x03) + unicode(str).  Each char in str
- * takes up two bytes in unicode format, so the resulting len(usbstr) is
- * 2 * len(str) + 2.
- * Returns the resulting length of the string in usbstr.
- * This function can accept overlapping usbstr and str as long as the overlap
- * does not cause data written to usbstr to overwrite data not yet read from
- * str.
- */
-static int make_usb_string(char *usbstr, size_t usblen, char *src,
-               size_t srclen)
-{
-       int len = 0;
-
-       if (usbstr && usblen >= 2 && src && *src && srclen) {
-               char *p;
-
-               if (usblen > 255)
-                       usblen = 255;
-
-               p = usbstr + 1;
-               *p++ = 0x03;
-               len = 2;
-               while (srclen && len < usblen) {
-                       *p++ = *src++;
-                       *p++ = 0;
-                       len += 2;
-                       srclen--;
-               }
-               *usbstr = (char)len;
-       }
-       return len;
-}
-
 /*
  * cp210x_setstr
  *
@@ -832,11 +838,9 @@ static int cp210x_ioctl(struct tty_struct *tty, struct file *file,
 
        case IOCTL_SETMFG:
        if (cp210x_has_setmfg()) {
-               char usbstr[CP210x_MAX_MFG_STRLEN * 2 + 2];
-               char *str = usbstr + sizeof(usbstr) - CP210x_MAX_MFG_STRLEN;
-               size_t len = cp210x_buf_from_user(str, arg,
-                               CP210x_MAX_MFG_STRLEN);
-               len = make_usb_string(usbstr, sizeof(usbstr), str, len);
+               char usbstr[USBSTRLEN(CP210x_MAX_MFG_STRLEN)];
+               size_t len = cp210x_usbstr_from_user(usbstr, arg,
+                               sizeof(usbstr));
                if (len && cp210x_setstr(port, 0x00, usbstr) == len)
                        return 0;
        }
@@ -845,11 +849,9 @@ static int cp210x_ioctl(struct tty_struct *tty, struct file *file,
 
        case IOCTL_SETPRODUCT:
        {
-               char usbstr[CP210x_MAX_PRODUCT_STRLEN * 2 + 2];
-               char *str = usbstr + sizeof(usbstr) - CP210x_MAX_PRODUCT_STRLEN;
-               size_t len = cp210x_buf_from_user(str, arg,
-                               CP210x_MAX_PRODUCT_STRLEN);
-               len = make_usb_string(usbstr, sizeof(usbstr), str, len);
+               char usbstr[USBSTRLEN(CP210x_MAX_PRODUCT_STRLEN)];
+               size_t len = cp210x_usbstr_from_user(usbstr, arg,
+                               sizeof(usbstr));
                if (len && cp210x_setstr(port, 0x03, usbstr) == len)
                        return 0;
                return -EFAULT;
@@ -858,11 +860,9 @@ static int cp210x_ioctl(struct tty_struct *tty, struct file *file,
 
        case IOCTL_SETSERIAL:
        {
-               char usbstr[CP210x_MAX_SERIAL_STRLEN * 2 + 2];
-               char *str = usbstr + sizeof(usbstr) - CP210x_MAX_SERIAL_STRLEN;
-               size_t len = cp210x_buf_from_user(str, arg,
-                               CP210x_MAX_SERIAL_STRLEN);
-               len = make_usb_string(usbstr, sizeof(usbstr), str, len);
+               char usbstr[USBSTRLEN(CP210x_MAX_SERIAL_STRLEN)];
+               size_t len = cp210x_usbstr_from_user(usbstr, arg,
+                               sizeof(usbstr));
                if (len && cp210x_setstr(port, 0x04, usbstr) == len)
                        return 0;
                return -EFAULT;

diff --git a/src/cp210x.h b/src/cp210x.h
index 9892464a93961a8a472097af290e7895308f3558..2de79d3077d1c1de2fe76e92c7eb4557ebda9fc3 100644 (file)
--- a/src/cp210x.h
+++ b/src/cp210x.h
@@ -47,7 +47,7 @@
 struct cp210x_buffer {
        __u8 *buf;
        __s32 len;
-} __attribute__((packed));
+};
 
 /* Port config definitions */
 struct cp210x_port_state {