On Mon, Jun 09 2014, Krzysztof Opasiak wrote: > 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? -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz (o o) ooo +--<mpn@xxxxxxxxxx>--<xmpp:mina86@xxxxxxxxxx>--ooO--(_)--Ooo-- -- 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