>> The name of this function has to be changed. Perhaps
>> ffs_do_os_single_desc? It took me embarrassing amount of time to figure
>> out that ffs_do_os_descs and ffs_do_os_desc are different functions.
On Mon, Jul 07 2014, Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx> wrote:
> Would you like me to add a patch which does the same thing to the
> "ffs_do_descs()/ffs_do_desc()" pair which has already been there?
Really? I did that... That's embarrassing. Yes, it would be nice.
>>> +#define FMT "invalid os descriptor length: %d pnl:%d pdl:%d (descriptor %d)\n"
>>> + pr_vdebug(FMT, length, pnl, pdl, type);
>>> +#undef FMT
>>
>> Uh? Why the #define?
> To avoid a line over 80 chars. Any better way?
>From coding style: “Statements longer than 80 columns will be broken
into sensible chunks, unless exceeding 80 columns significantly
increases readability and does not hide information.” In this instance
the exception applies, so do:
pr_vdebug("invalid os descriptor length: %d pnl:%d pdl:%d (descriptor %d)\n",
length, pnl, pdl, type);
If you *really* dislike it (but I actually prefer not breaking string
literals as it helps with greping the sources) you can just break the
string literal into parts:
pr_vdebug("invalid os descriptor length: %d pnl:%d "
"pdl:%d (descriptor %d)\n",
length, pnl, pdl, type);
>>> + return -EINVAL;
>>> + }
>>> + ++ffs->ms_os_descs_ext_prop_count;
>>> + ffs->ms_os_descs_ext_prop_name_len += (pnl << 1);
>>
>> Why is pnl multiplied by two?
>
> Strings in Feature Descriptors (Extended Properties, Extended Compatibility)
> are returned to the host as "WCHAR"s, and so "property name length" must
> reflect this.
I'd add a comment to that effect, and do “pnl * 2” instead of “pnl << 1”
to make it clear it's multiplication. Compiler is smart enough to
optimise it the best way possible.
--
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
