Re: [PATCH v2] usb: gadget: OS desc type unicode multi

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Mario,

W dniu 01.02.2015 o 21:07, Mario Schuknecht pisze:
A popular proprietary operating system expects that USB devices provide extra
information via "OS descriptors". An introduction to the subject can be found
here:


<snip>


This patch adds support for property data type 0x7 multiple NUL-terminated
unicode strings.

This suggests that you mean

#define USB_EXT_PROP_UNICODE_MULTI		7

Add case USB_EXT_PROP_UNICODE_MULTI in function fill_ext_prop()

And you do.


Signed-off-by: Mario Schuknecht <mario.schuknecht@xxxxxxxxxxxxxxx>
---

Changes in v2:
- improve commit log

  drivers/usb/gadget/composite.c |  5 +++++
  drivers/usb/gadget/u_os_desc.h | 28 ++++++++++++++++++++++++++++
  2 files changed, 33 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 6178353..bf30b73 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1422,6 +1422,11 @@ static int fill_ext_prop(struct usb_configuration *c, int interface, u8 *buf)
  							 ext_prop->data,
  							 ext_prop->data_len);
  					break;
+				case USB_EXT_PROP_UNICODE_LINK_MULTI:
Does it compile? I mean the _LINK_ infix - didn't you mean

+				case USB_EXT_PROP_UNICODE_MULTI:

instead?

+					usb_ext_prop_put_unicode_multi(buf, ret,
+							 ext_prop->data,
+							 ext_prop->data_len);
+					break;
  				case USB_EXT_PROP_BINARY:
  					usb_ext_prop_put_binary(buf, ret,
  							ext_prop->data,
diff --git a/drivers/usb/gadget/u_os_desc.h b/drivers/usb/gadget/u_os_desc.h
index 947b7dd..bee4274 100644
--- a/drivers/usb/gadget/u_os_desc.h
+++ b/drivers/usb/gadget/u_os_desc.h
@@ -120,4 +120,32 @@ static inline int usb_ext_prop_put_unicode(u8 *buf, int pnl, const char *string,
  	return data_len;
  }


I'm not sure I understand the logic of the below function well.

+static inline int usb_ext_prop_put_unicode_multi(u8 *buf, int pnl,
+					const char *string, int data_len)
+{
+	int outlen = 0;
+
+	put_unaligned_le32(data_len, usb_ext_prop_data_len_ptr(buf, pnl));
+
+	while (*string && outlen < data_len - 2) {

You keep looping as long as the source *string is not '\0'
and advance the string in each loop by adding (strlen() + 1) of
what is currently available starting at *string.
For example:

string: "a\0b\0and this is past the end of your source buffer"

first loop iteration:
len = strlen(string); /* len == 1 */
string += len + 1; /* string: "b\0and this is past the end of your source buffer" */

second loop iteration:
len = strlen(string); /* len == 1 */
string += len + 1; /* string: "and this is past the end of your source buffer" */

so effectively the first part of the while condition rarely ever becomes "false".
In other words when you process all the source strings from "string" you, by design,
end up one byte past the terminating '\0' of the source buffer. The contents
of this memory can be anything, there is just 1/256 chance it is zero,
so the "while (*string" part does not make sense to me.


+		int len = strlen(string);
+		int result = utf8s_to_utf16s(string, len, UTF16_LITTLE_ENDIAN,
+			(wchar_t *) usb_ext_prop_data_ptr(buf, pnl + outlen),
+			data_len - outlen - 2);
+		if (result < 0)
+			return result;
+		outlen += result << 1;
+		string += len + 1;
+		put_unaligned_le16(0,
+			&buf[USB_EXT_PROP_B_PROPERTY_DATA + pnl + outlen]);
You put a terminating NUL "two-byte" after each destination, "wide" string.

+		outlen += 2;

Then you advance the "outlen" to reflect the above.


+	}
Suppose there is just one string in the source "string", so the loop
terminates now.

Am I correct that at this point "outlen" is supposed to be equal "data_len"?
If it is, than please see below (*).


+
+	put_unaligned_le16(0,
+			&buf[USB_EXT_PROP_B_PROPERTY_DATA + pnl + outlen]);

Why again terminate the destination string? And, since outlen value was
incremented by 2 in the meantime there will be two terminators in a row?

+	outlen += 2;

(*)
If "outlen" was supposed to be equal "data_len" above then the
return value of the whole function becomes data_len + 2.

+
+	return outlen;
+}
+
  #endif /* __U_OS_DESC_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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux