Hi Michal,
> -----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 ...)
> +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;
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.
> +
> +#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;
> +}
> +
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]
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
