[4/13](resend)hidclass.sys: Add USB Descriptor parsing

Henri Verbeet hverbeet at gmail.com
Thu Sep 3 07:52:45 CDT 2015


On 3 September 2015 at 14:09, Aric Stewart <aric at codeweavers.com> wrote:
>> Here and elsewhere, there's a lot of magic numbers. Do these have
>> names in the standard? Could you make up names for them? TAG_INPUT or
>> something. Or is it more useful to have the raw numbers?
>
> All these magic number have defined tags as per the USB spec. I was really hoping to find a nice header file where they where all defined but while there are a lot of document describing them and tools to turn the tags into numbers there was not clean header file. I chose to keep the numbers as numbers to avoid a lot of #defines. But that was just a personal choice.

You could use an enum, of course.

> +static const char* feature_string[] =
You probably meant to write "static const char * const
feature_string[] =" here. (And probably in some other places where you
have arrays of string pointers.)

> +        return wine_dbg_sprintf("[%x - %x]", caps->u.Range.UsageMin, caps->u.Range.UsageMax);
You almost never want to use plain "%x". (E.g., does "20" mean 32 or
20?) Use either "%#x or 0x%08x.

> +int parse_descriptor(BYTE *descriptor, INT index, INT length, INT *feature_index, INT *collection_index, struct collection *collection, struct caps *caps, struct list *features)
> +{
> +    int i;
> +    for (i = index; i < length;)
Unless "index" can legitimitely be < 0, it should be unsigned. I think
the applies to most cases where you're using signed types.

> +        data->dwFeatureReportCount ++;
That's a really unconventional way to format this.



More information about the wine-devel mailing list