lkml.org 
[lkml]   [2014]   [Jun]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCHv2 5/5] tools: ffs-test: add compatibility code for old kernels
Date
Hi Michal,

> -----Original Message-----
> From: Michal Nazarewicz [mailto:mina86@mina86.com]
> Sent: Friday, June 06, 2014 11:13 AM
> To: Felipe Balbi; Krzysztof Opasiak
> Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; 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
k.opasiak@samsung.com






\
 
 \ /
  Last update: 2014-06-09 11:21    [W:0.075 / U:0.424 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site