Skip to contentSkip to conversation

usb: gadget: f_fs: resurect usb_functionfs_descs_head structure

PATCH Superseded Michal Nazarewicz 2 participants 9 messages Jun 6, 2014 – Jun 9, 2014

Subsystems (2)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/5 usb: gadget: f_fs: resurect usb_functionfs_descs_head structure

Even though usb_functionfs_descs_head structure is now deprecated, it has been used by some user space tools. Its removel in commit [ac8dde1: “Add flags to descriptors block”] was an oversight leading to build breakage for such tools.

Bring it back so that old user space tools can still be build without problems on newer kernel versions.

---
 include/uapi/linux/usb/functionfs.h | 7 +++++++
 1 file changed, 7 insertions(+)
include/uapi/linux/usb/functionfs.h +7 -0
index 2a4b4a7..24b68c5 100644
@@ -33,6 +33,13 @@ struct usb_endpoint_descriptor_no_audio {
 	__u8  bInterval;
 } __attribute__((packed));
 
+/* Legacy format, deprecated as of 3.14. */
+struct usb_functionfs_descs_head {
+	__le32 magic;
+	__le32 length;
+	__le32 fs_count;
+	__le32 hs_count;
+} __attribute__((packed, deprecated));
 
 /*
  * Descriptors format:
Signature · 1 line
2.0.0.526.g5318336
#1 lore raw

PATCH v2 2/5 tools: ffs-test: fix header values endianess

It appears that no one ever run ffs-test on a big-endian machine, since it used cpu-endianess for fs_count and hs_count fields which should be in little-endian format. Fix by wrapping the numbers in cpu_to_le32.

---
 tools/usb/ffs-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/usb/ffs-test.c +2 -2
index fe1e66b..a87e99f 100644
@@ -116,8 +116,8 @@ static const struct {
 	.header = {
 		.magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC),
 		.length = cpu_to_le32(sizeof descriptors),
-		.fs_count = 3,
-		.hs_count = 3,
+		.fs_count = cpu_to_le32(3),
+		.hs_count = cpu_to_le32(3),
 	},
 	.fs_descs = {
 		.intf = {
Signature · 1 line
2.0.0.526.g5318336
#2 lore raw

PATCH v2 3/5 usb: gadget: f_fs: add usb_functionfs_descs_head_v2 structure

In reply to Michal Nazarewicz

The structure can be used with user space tools that use the new functionfs description format, for example as follows:

static const struct {
	struct usb_functionfs_descs_head_v2 header;
	__le32 fs_count;
	__le32 hs_count;
	struct {
		…
	} fs_desc;
	struct {
		…
	} hs_desc;
} descriptors = {
	.header = {
		.magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC_V2),
		.length = cpu_to_le32(sizeof(descriptors)),
		.flags = cpu_to_le32(FUNCTIONFS_HAS_FS_DESC |
				     FUNCTIONFS_HAS_HS_DESC)
	},
	.fs_count = cpu_to_le32(X),
	.fs_desc = {
		…
	},
	.hs_count = cpu_to_le32(Y),
	.hs_desc = {
		…
	}
};
---
 include/uapi/linux/usb/functionfs.h | 10 ++++++++++
 1 file changed, 10 insertions(+)
include/uapi/linux/usb/functionfs.h +10 -0
index 24b68c5..2592d86 100644
@@ -33,6 +33,16 @@ struct usb_endpoint_descriptor_no_audio {
 	__u8  bInterval;
 } __attribute__((packed));
 
+struct usb_functionfs_descs_head_v2 {
+	__le32 magic;
+	__le32 length;
+	__le32 flags;
+	/*
+	 * __le32 fs_count, hs_count, fs_count; must be included manually in
+	 * the structure taking flags into consideration.
+	 */
+} __attribute__((packed));
+
 /* Legacy format, deprecated as of 3.14. */
 struct usb_functionfs_descs_head {
 	__le32 magic;
Signature · 1 line
2.0.0.526.g5318336
#3 lore raw

PATCH v2 4/5 tools: ffs-test: convert to new descriptor format

In reply to Michal Nazarewicz

Since commit [ac8dde11: “Add flags to descriptors block”] functionfs supports a new, more powerful and extensible, descriptor format. Since ffs-test is probably the first thing users of the functionfs interface see when they start writing functionfs user space daemons, convert it to use the new format thus promoting it.

---
 include/uapi/linux/usb/functionfs.h |  2 +-
 tools/usb/ffs-test.c                | 14 +++++++++-----
 2 files changed, 10 insertions(+), 6 deletions(-)
include/uapi/linux/usb/functionfs.h +1 -1
index 2592d86..1dc473a 100644
@@ -70,7 +70,7 @@ struct usb_functionfs_descs_head {
  * structure.  Any flags that are not recognised cause the whole block to be
  * rejected with -ENOSYS.
  *
- * Legacy descriptors format:
+ * Legacy descriptors format (deprecated as of 3.14):
  *
  * | off | name      | type         | description                          |
  * |-----+-----------+--------------+--------------------------------------|
tools/usb/ffs-test.c +9 -5
index a87e99f..708d317 100644
@@ -1,5 +1,5 @@
 /*
- * ffs-test.c.c -- user mode filesystem api for usb composite function
+ * ffs-test.c -- user mode filesystem api for usb composite function
  *
  * Copyright (C) 2010 Samsung Electronics
  *                    Author: Michal Nazarewicz <[email protected]>
@@ -106,7 +106,9 @@ static void _msg(unsigned level, const char *fmt, ...)
 /******************** Descriptors and Strings *******************************/
 
 static const struct {
-	struct usb_functionfs_descs_head header;
+	struct usb_functionfs_descs_head_v2 header;
+	__le32 fs_count;
+	__le32 hs_count;
 	struct {
 		struct usb_interface_descriptor intf;
 		struct usb_endpoint_descriptor_no_audio sink;
@@ -114,11 +116,12 @@ static const struct {
 	} __attribute__((packed)) fs_descs, hs_descs;
 } __attribute__((packed)) descriptors = {
 	.header = {
-		.magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC),
+		.magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC_V2),
+		.flags = cpu_to_le32(FUNCTIONFS_HAS_FS_DESC |
+				     FUNCTIONFS_HAS_HS_DESC),
 		.length = cpu_to_le32(sizeof descriptors),
-		.fs_count = cpu_to_le32(3),
-		.hs_count = cpu_to_le32(3),
 	},
+	.fs_count = cpu_to_le32(3),
 	.fs_descs = {
 		.intf = {
 			.bLength = sizeof descriptors.fs_descs.intf,
@@ -142,6 +145,7 @@ static const struct {
 			/* .wMaxPacketSize = autoconfiguration (kernel) */
 		},
 	},
+	.hs_count = cpu_to_le32(3),
 	.hs_descs = {
 		.intf = {
 			.bLength = sizeof descriptors.fs_descs.intf,
Signature · 1 line
2.0.0.526.g5318336
#4 lore raw

PATCH v2 5/5 tools: ffs-test: add compatibility code for old kernels

In reply to Michal Nazarewicz

If ffs-test is used with a kernel prior to 3.14, which do not support the new descriptors format, it will fail when trying to write the descriptors. Add a function that converts the new descriptors to the legacy ones and use it to retry writing the descriptors using the legacy format.

Also add “-l” flag to ffs-test which will cause the tool to never try the new format and instead immediatelly try the legacy one. This should be useful to test whether parsing of the old format still works on given 3.14+ kernel.

---
 tools/usb/ffs-test.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 79 insertions(+), 5 deletions(-)
tools/usb/ffs-test.c +79 -5
index 708d317..407c828 100644
@@ -29,6 +29,7 @@
 #include <fcntl.h>
 #include <pthread.h>
 #include <stdarg.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -172,6 +173,61 @@ static const struct {
 	},
 };
 
+static size_t descs_to_legacy(void **legacy, const void *descriptors) {
+	__u32 length, flags, fs_count = 0, hs_count = 0, count;
+	const unsigned char *descs = descriptors, *usb_descs;
+	unsigned char *out;
+
+	if (get_unaligned_le32(descs) != FUNCTIONFS_DESCRIPTORS_MAGIC_V2)
+		return 0;
+
+	length = get_unaligned_le32(descs + 4);
+	if (length < 8)
+		return 0;
+	descs += 8;
+	length -= 8;
+
+#define GET_LE32(ret) do {				\
+		if (length < 4)				\
+			return 0;			\
+		ret = get_unaligned_le32(descs);	\
+		descs += 4;				\
+		length -= 4;				\
+	} while (0)
+
+	GET_LE32(flags);
+	if (flags & FUNCTIONFS_HAS_FS_DESC)
+		GET_LE32(fs_count);
+	if (flags & FUNCTIONFS_HAS_HS_DESC)
+		GET_LE32(hs_count);
+	if (!fs_count && !hs_count)
+		return 0;
+	if (flags & FUNCTIONFS_HAS_SS_DESC)
+		GET_LE32(count);  /* The value is ignored later on anyway. */
+	if (flags)
+		return 0;
+
+#undef GET_LE32
+
+	usb_descs = descs;
+	for (count = fs_count + hs_count; count; --count) {
+		if (length < *descs)
+			return 0;
+		length -= *descs;
+		descs += *descs;
+	}
+
+	length = 16 + (descs - usb_descs);
+	out = malloc(length);
+	put_unaligned_le32(FUNCTIONFS_DESCRIPTORS_MAGIC, out);
+	put_unaligned_le32(length, out + 4);
+	put_unaligned_le32(fs_count, out + 8);
+	put_unaligned_le32(hs_count, out + 12);
+	memcpy(out + 16, usb_descs, length - 16);
+	*legacy = out;
+	return length;
+}
+
 
 #define STR_INTERFACE_ "Source/Sink"
 
@@ -491,12 +547,29 @@ ep0_consume(struct thread *ignore, const void *buf, size_t nbytes)
 	return nbytes;
 }
 
-static void ep0_init(struct thread *t)
+static void ep0_init(struct thread *t, bool legacy_descriptors)
 {
+	void *legacy;
 	ssize_t ret;
+	size_t len;
+
+	if (legacy_descriptors) {
+		info("%s: writing descriptors\n", t->filename);
+		goto legacy;
+	}
 
-	info("%s: writing descriptors\n", t->filename);
+	info("%s: writing descriptors (in v2 format)\n", t->filename);
 	ret = write(t->fd, &descriptors, sizeof descriptors);
+
+	if (ret < 0 && errno == EINVAL) {
+		warn("%s: new format rejected, trying legacy\n", t->filename);
+legacy:
+		len = descs_to_legacy(&legacy, &descriptors);
+		if (len) {
+			ret = write(t->fd, legacy, len);
+			free(legacy);
+		}
+	}
 	die_on(ret < 0, "%s: write: descriptors", t->filename);
 
 	info("%s: writing strings\n", t->filename);
@@ -507,14 +580,15 @@ static void ep0_init(struct thread *t)
 
 /******************** Main **************************************************/
 
-int main(void)
+int main(int argc, char **argv)
 {
+	bool legacy_descriptors;
 	unsigned i;
 
-	/* XXX TODO: Argument parsing missing */
+	legacy_descriptors = argc > 2 && !strcmp(argv[1], "-l");
 
 	init_thread(threads);
-	ep0_init(threads);
+	ep0_init(threads, legacy_descriptors);
 
 	for (i = 1; i < sizeof threads / sizeof *threads; ++i)
 		init_thread(threads + i);
Signature · 1 line
2.0.0.526.g5318336
#5 lore raw

RE: [PATCHv2 5/5] tools: ffs-test: add compatibility code for old kernels

Hi Michal,

Quoted 8 lines

-----Original Message-----
From: Michal Nazarewicz [mailto:[email protected]]
Sent: Friday, June 06, 2014 11:13 AM
To: Felipe Balbi; Krzysztof Opasiak
Cc: [email protected]; [email protected]; Michal Nazarewicz
Subject: [PATCHv2 5/5] tools: ffs-test: add compatibility code for old kernels

(... snip ...)

Quoted 34 lines

+static size_t descs_to_legacy(void **legacy, const void *descriptors) {

tools/usb/ffs-test.c +5 -0
+	__u32 length, flags, fs_count = 0, hs_count = 0, count;
+	const unsigned char *descs = descriptors, *usb_descs;
+	unsigned char *out;
+
+	if (get_unaligned_le32(descs) !=

FUNCTIONFS_DESCRIPTORS_MAGIC_V2)

tools/usb/ffs-test.c +25 -0
+		return 0;
+
+	length = get_unaligned_le32(descs + 4);
+	if (length < 8)
+		return 0;
+	descs += 8;
+	length -= 8;
+
+#define GET_LE32(ret) do {				\
+		if (length < 4)				\
+			return 0;			\
+		ret = get_unaligned_le32(descs);	\
+		descs += 4;				\
+		length -= 4;				\
+	} while (0)
+
+	GET_LE32(flags);
+	if (flags & FUNCTIONFS_HAS_FS_DESC)
+		GET_LE32(fs_count);
+	if (flags & FUNCTIONFS_HAS_HS_DESC)
+		GET_LE32(hs_count);
+	if (!fs_count && !hs_count)
+		return 0;
+	if (flags & FUNCTIONFS_HAS_SS_DESC)
+		GET_LE32(count);  /* The value is ignored later on

anyway. */

Review

Quoted 2 lines
(quoted diff) +2 -0
+	if (flags)
+		return 0;

As far as I understand you are taking the flags from descriptor and then test them with possible ffs speed flags, after getting flags your are not assigning anything to this variable so in this place it will be != 0 unless none of the flags has been provided. I don't think this is intended behavior.

Quoted 12 lines
tools/usb/ffs-test.c +12 -0
+
+#undef GET_LE32
+
+	usb_descs = descs;
+	for (count = fs_count + hs_count; count; --count) {
+		if (length < *descs)
+			return 0;
+		length -= *descs;
+		descs += *descs;
+	}
+
+	length = 16 + (descs - usb_descs);

tools/usb/ffs-test.c static const struct { · line 230

Quoted 10 lines
tools/usb/ffs-test.c +10 -0
+	out = malloc(length);
+	put_unaligned_le32(FUNCTIONFS_DESCRIPTORS_MAGIC, out);
+	put_unaligned_le32(length, out + 4);
+	put_unaligned_le32(fs_count, out + 8);
+	put_unaligned_le32(hs_count, out + 12);
+	memcpy(out + 16, usb_descs, length - 16);
+	*legacy = out;
+	return length;
+}
+

As this is an example which will be copy-paste by many users maybe you should you struct usb_functionfs_descs_head and struct usb_functionfs_descs_head_v2 instead of direct operations using hard-coded offsets to make this function more readable?

--
BR's
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
[email protected]

#6 lore raw

PATCH v3 1/1 tools: ffs-test: add compatibility code for old kernels

If ffs-test is used with a kernel prior to 3.14, which do not support the new descriptors format, it will fail when trying to write the descriptors. Add a function that converts the new descriptors to the legacy ones and use it to retry writing the descriptors using the legacy format.

Also add “-l” flag to ffs-test which will cause the tool to never try the new format and instead immediatelly try the legacy one. This should be useful to test whether parsing of the old format still works on given 3.14+ kernel.

---
 tools/usb/ffs-test.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 100 insertions(+), 5 deletions(-)
tools/usb/ffs-test.c +100 -5
index 708d317..afd69ea 100644
@@ -29,6 +29,7 @@
 #include <fcntl.h>
 #include <pthread.h>
 #include <stdarg.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -172,6 +173,82 @@ static const struct {
 	},
 };
 
+static size_t descs_to_legacy(void **legacy, const void *descriptors) {
+	__u32 length, flags, fs_count = 0, hs_count = 0, count;
+	const unsigned char *descs, *usb_descs;
+
+	/* Read v2 header */
+	{
+		const struct usb_functionfs_descs_head_v2 *head = descriptors;
+
+		if (le32_to_cpu(head->magic) !=
+		    FUNCTIONFS_DESCRIPTORS_MAGIC_V2)
+			return 0;
+		length = le32_to_cpu(head->length);
+		if (length <= sizeof *head)
+			return 0;
+		flags = le32_to_cpu(head->flags);
+		descs = (const void *)(head + 1);
+		length -= sizeof *head;
+	}
+
+	/* Read v2 counts */
+#define GET_LE32(ret) do {					\
+		if (length < 4)					\
+			return 0;				\
+		ret = le32_to_cpu(*(const __le32 *)descs);	\
+		descs += 4;					\
+		length -= 4;					\
+	} while (0)
+
+	if (flags & FUNCTIONFS_HAS_FS_DESC)
+		GET_LE32(fs_count);
+	if (flags & FUNCTIONFS_HAS_HS_DESC)
+		GET_LE32(hs_count);
+	if (!fs_count && !hs_count)
+		return 0;
+	if (flags & FUNCTIONFS_HAS_SS_DESC)
+		/* Legacy does not support SS so this is ignored. */
+		GET_LE32(count);
+	if (flags & ~(FUNCTIONFS_HAS_FS_DESC | FUNCTIONFS_HAS_HS_DESC |
+		      FUNCTIONFS_HAS_SS_DESC))
+		return 0;
+
+#undef GET_LE32
+
+	/*
+	 * Find the end of FS and HS USB descriptors.  SS descriptors
+	 * are ignored since legacy format does not support them.
+	 */
+	count = fs_count + hs_count;
+	usb_descs = descs;
+	do {
+		if (length < *descs)
+			return 0;
+		length -= *descs;
+		descs += *descs;
+	} while (--count);
+
+	/* Allocate legacy descriptors and copy the data. */
+	{
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+		struct usb_functionfs_descs_head *head;
+#pragma GCC diagnostic pop
+
+		length = sizeof *head + (descs - usb_descs);
+		head = malloc(length);
+		head->magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC);
+		head->length = cpu_to_le32(length);
+		head->fs_count = cpu_to_le32(fs_count);
+		head->hs_count = cpu_to_le32(hs_count);
+		memcpy(head + 1, usb_descs, descs - usb_descs);
+		*legacy = head;
+	}
+
+	return length;
+}
+
 
 #define STR_INTERFACE_ "Source/Sink"
 
@@ -491,12 +568,29 @@ ep0_consume(struct thread *ignore, const void *buf, size_t nbytes)
 	return nbytes;
 }
 
-static void ep0_init(struct thread *t)
+static void ep0_init(struct thread *t, bool legacy_descriptors)
 {
+	void *legacy;
 	ssize_t ret;
+	size_t len;
+
+	if (legacy_descriptors) {
+		info("%s: writing descriptors\n", t->filename);
+		goto legacy;
+	}
 
-	info("%s: writing descriptors\n", t->filename);
+	info("%s: writing descriptors (in v2 format)\n", t->filename);
 	ret = write(t->fd, &descriptors, sizeof descriptors);
+
+	if (ret < 0 && errno == EINVAL) {
+		warn("%s: new format rejected, trying legacy\n", t->filename);
+legacy:
+		len = descs_to_legacy(&legacy, &descriptors);
+		if (len) {
+			ret = write(t->fd, legacy, len);
+			free(legacy);
+		}
+	}
 	die_on(ret < 0, "%s: write: descriptors", t->filename);
 
 	info("%s: writing strings\n", t->filename);
@@ -507,14 +601,15 @@ static void ep0_init(struct thread *t)
 
 /******************** Main **************************************************/
 
-int main(void)
+int main(int argc, char **argv)
 {
+	bool legacy_descriptors;
 	unsigned i;
 
-	/* XXX TODO: Argument parsing missing */
+	legacy_descriptors = argc > 2 && !strcmp(argv[1], "-l");
 
 	init_thread(threads);
-	ep0_init(threads);
+	ep0_init(threads, legacy_descriptors);
 
 	for (i = 1; i < sizeof threads / sizeof *threads; ++i)
 		init_thread(threads + i);
Signature · 1 line
2.0.0.526.g5318336
#7 lore raw

Re: [PATCHv2 5/5] tools: ffs-test: add compatibility code for old kernels

In reply to Krzysztof Opasiak
Quoted 4 lines · Krzysztof Opasiak

As this is an example which will be copy-paste by many users maybe you should you struct usb_functionfs_descs_head and struct usb_functionfs_descs_head_v2 instead of direct operations using hard-coded offsets to make this function more readable?

v3 uses usb_functionfs_descs_head{_v2,} instead of hard-coded offsets. I also started wondering if it would make sense to go one step further and define a temporary structure holding the headers. Something like:

----------- >8 ---------------------------------------------------------

	/* Read v2 header */
	{
		const struct {
			const struct usb_functionfs_descs_head_v2 header;
			const __le32 counts[];
		} __attribute__((packed)) *const in = descriptors;
		const __le32 *counts = in->counts;
		__u32 flags;

		if (le32_to_cpu(in->header.magic) !=
		    FUNCTIONFS_DESCRIPTORS_MAGIC_V2)
			return 0;
		length = le32_to_cpu(in->header.length);
		if (length <= sizeof in->header)
			return 0;
		length -= sizeof in->header;
		flags = le32_to_cpu(in->header.flags);
		if (flags & ~(FUNCTIONFS_HAS_FS_DESC | FUNCTIONFS_HAS_HS_DESC |
			      FUNCTIONFS_HAS_SS_DESC))
			return 0;
#define GET_NEXT_COUNT_IF_FLAG(ret, flg) do {		\
			if (!(flags & (flg)))		\
				break;			\
			if (length < 4)			\
				return 0;		\
			ret = le32_to_cpu(*counts);	\
			length -= 4;			\
			++counts;			\
		} while (0)

		GET_NEXT_COUNT_IF_FLAG(fs_count, FUNCTIONFS_HAS_FS_DESC);
		GET_NEXT_COUNT_IF_FLAG(hs_count, FUNCTIONFS_HAS_HS_DESC);
		GET_NEXT_COUNT_IF_FLAG(count, FUNCTIONFS_HAS_SS_DESC);

		count = fs_count + hs_count;
		if (!count)
			return 0;
		descs_start = descs_end = (const void *)counts;

#undef GET_NEXT_COUNT_IF_FLAG

	}

----------- >8 ---------------------------------------------------------

	/* Allocate legacy descriptors and copy the data. */
	{
		struct {
			struct usb_functionfs_descs_head header;
			__u8 descriptors[];
		} __attribute__((packed)) *out;

		length = sizeof out->header + (descs_end - descs_start);
		out = malloc(length);
		out->header.magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC);
		out->header.length = cpu_to_le32(length);
		out->header.fs_count = cpu_to_le32(fs_count);
		out->header.hs_count = cpu_to_le32(hs_count);
		memcpy(out->descriptors, descs_start, descs_end - descs_start);
		*legacy = out;
	}

----------- >8 ---------------------------------------------------------

Thoughts?

Signature · 4 lines
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<[email protected]>--<xmpp:[email protected]>--ooO--(_)--Ooo--
#8 lore raw

RE: [PATCHv2 5/5] tools: ffs-test: add compatibility code for old kernels

Hi,

Quoted 93 lines

-----Original Message-----
From: Michal Nazarewicz [mailto:[email protected]] On Behalf Of Michal Nazarewicz
Sent: Monday, June 09, 2014 3:28 PM
To: Krzysztof Opasiak; 'Felipe Balbi'; Krzysztof Opasiak
Cc: [email protected]; [email protected]
Subject: Re: [PATCHv2 5/5] tools: ffs-test: add compatibility code for old kernels

Quoted 1 line · Krzysztof Opasiak

As this is an example which will be copy-paste by many users

maybe you

Quoted 3 lines

should you struct usb_functionfs_descs_head and struct usb_functionfs_descs_head_v2 instead of direct operations using hard-coded offsets to make this function more readable?

v3 uses usb_functionfs_descs_head{_v2,} instead of hard-coded offsets.
I also started wondering if it would make sense to go one step further
and define a temporary structure holding the headers. Something like:

----------- >8 ----------------------------------------------------
-----

	/* Read v2 header */
	{
		const struct {
			const struct usb_functionfs_descs_head_v2 header;
			const __le32 counts[];
		} __attribute__((packed)) *const in = descriptors;
		const __le32 *counts = in->counts;
		__u32 flags;

		if (le32_to_cpu(in->header.magic) !=
		    FUNCTIONFS_DESCRIPTORS_MAGIC_V2)
			return 0;
		length = le32_to_cpu(in->header.length);
		if (length <= sizeof in->header)
			return 0;
		length -= sizeof in->header;
		flags = le32_to_cpu(in->header.flags);
		if (flags & ~(FUNCTIONFS_HAS_FS_DESC |
FUNCTIONFS_HAS_HS_DESC |
			      FUNCTIONFS_HAS_SS_DESC))
			return 0;
#define GET_NEXT_COUNT_IF_FLAG(ret, flg) do {		\
			if (!(flags & (flg)))		\
				break;			\
			if (length < 4)			\
				return 0;		\
			ret = le32_to_cpu(*counts);	\
			length -= 4;			\
			++counts;			\
		} while (0)

		GET_NEXT_COUNT_IF_FLAG(fs_count,
FUNCTIONFS_HAS_FS_DESC);
		GET_NEXT_COUNT_IF_FLAG(hs_count,
FUNCTIONFS_HAS_HS_DESC);
		GET_NEXT_COUNT_IF_FLAG(count, FUNCTIONFS_HAS_SS_DESC);

		count = fs_count + hs_count;
		if (!count)
			return 0;
		descs_start = descs_end = (const void *)counts;

#undef GET_NEXT_COUNT_IF_FLAG

	}

----------- >8 ----------------------------------------------------
-----

	/* Allocate legacy descriptors and copy the data. */
	{
		struct {
			struct usb_functionfs_descs_head header;
			__u8 descriptors[];
		} __attribute__((packed)) *out;

		length = sizeof out->header + (descs_end -
descs_start);
		out = malloc(length);
		out->header.magic =
cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC);
		out->header.length = cpu_to_le32(length);
		out->header.fs_count = cpu_to_le32(fs_count);
		out->header.hs_count = cpu_to_le32(hs_count);
		memcpy(out->descriptors, descs_start, descs_end -
descs_start);
		*legacy = out;
	}
Quoted 4 lines

----------- >8 ----------------------------------------------------
-----

Thoughts?

Looks very good to me! In my opinion this one should be used, it emphasizes your intentions very well so it's perfect code for sample app.

--
BR's
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
[email protected]