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

[Andrew Eikum]

A couple small notes inline below.

On Wed, Sep 02, 2015 at 07:01:51AM -0500, Aric Stewart wrote:
> diff --git a/dlls/hidclass.sys/Makefile.in b/dlls/hidclass.sys/Makefile.in
> index 8ed51e6..6e79486 100644
> --- a/dlls/hidclass.sys/Makefile.in
> +++ b/dlls/hidclass.sys/Makefile.in
> @@ -5,6 +5,7 @@ DELAYIMPORTS = setupapi hid
>
> C_SRCS = \
>    buffer.c \
> - descriptor.c \
> device.c \
> main.c \
> pnp.c
>
> diff --git a/dlls/hidclass.sys/descriptor.c b/dlls/hidclass.sys/descriptor.c
> new file mode 100644
> index 0000000..c277dec
> --- /dev/null
> +++ b/dlls/hidclass.sys/descriptor.c
> @@ -0,0 +1,1106 @@
> +/*
> -
> - HID descriptor parsing
> -
> -
> -
> - Copyright (C) 2015 Aric Stewart
> -
> -
> -
> - This library is free software; you can redistribute it and/or
> -
> - modify it under the terms of the GNU Lesser General Public
> -
> - License as published by the Free Software Foundation; either
> -
> - version 2.1 of the License, or (at your option) any later version.
> -
> -
> -
> - This library is distributed in the hope that it will be useful,
> -
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> -
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -
> - Lesser General Public License for more details.
> -
> -
> -
> - You should have received a copy of the GNU Lesser General Public
> -
> - License along with this library; if not, write to the Free Software
> -
> - Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
> - */
> -
>
> +#include "config.h"
> -
>
> +#include <stdarg.h>
> +#include <stdlib.h>
> +#define NONAMELESSUNION
> +#include "hid.h"
> -
>
> +#include "wine/debug.h"
> +#include "wine/list.h"
> -
>
> +WINE_DEFAULT_DEBUG_CHANNEL(hid);
> -
>
> +#define USAGE_MAX 10
> -
>
> +static const char* feature_string[] =
> - { "Input", "Output", "Feature" };
> -
>
> +struct caps {
> - USAGE UsagePage;
> - LONG LogicalMin;
> - LONG LogicalMax;
> - LONG PhysicalMin;
> - LONG PhysicalMax;
> - ULONG UnitsExp;
> - ULONG Units;
> - USHORT BitSize;
> - UCHAR ReportID;
> - USHORT ReportCount;
> -
> - BOOLEAN  IsRange;
> - BOOLEAN  IsStringRange;
> - BOOLEAN  IsDesignatorRange;
> - int usage_count;
> - union {
> -
>    struct {
> -
>        USAGE UsageMin;
> -
>        USAGE UsageMax;
> -
>        USHORT StringMin;
> -
>        USHORT StringMax;
> -
>        USHORT DesignatorMin;
> -
>        USHORT DesignatorMax;
> -
>    } Range;
> -
>    struct  {
> -
>        USAGE Usage[USAGE_MAX];
> -
>        USAGE Reserved1;
> -
>        USHORT StringIndex;
> -
>        USHORT Reserved2;
> -
>        USHORT DesignatorIndex;
> -
>        USHORT Reserved3;
> -
>    } NotRange;
> - } DUMMYUNIONNAME;
> -
> - int Delim;
>
> +};
> -
>
> +struct feature {
> - struct list entry;
> - struct list col_entry;
> - struct caps caps;
> -
> - HIDP_REPORT_TYPE type;
> - BOOLEAN isData;
> - BOOLEAN isArray;
> - BOOLEAN IsAbsolute;
> - BOOLEAN Wrap;
> - BOOLEAN Linear;
> - BOOLEAN prefState;
> - BOOLEAN HasNull;
> - BOOLEAN Volatile;
> - BOOLEAN BitField;
> -
> - int index;
> - struct collection *collection;
>
> +};
> -
>
> +static const char* collection_string[] = {
> - "Physical",
> - "Application",
> - "Logical",
> - "Report",
> - "Named Array",
> - "Usage Switch",
> - "Usage Modifier",
>
> +};
> -
>
> +struct collection {
> - struct list entry;
> - struct caps caps;
> - int index;
> - int type;
> - struct collection *parent;
> - struct list features;
> - struct list collections;
>
> +};
> -
>
> +static const char* debugstr_usages(struct caps *caps)
> +{
> - if (!caps->IsRange)
> - {
> -
>    static char out[30];
> -
>    int i;
> -
>    strcpy(out, "[ ");
> -
>    for (i = 0; i < caps->usage_count; i++)
> -
>        strcat(out, wine_dbg_sprintf("%x ", caps->u.NotRange.Usage[i]));
> -
>    strcat(out,"]");
> -
>    return out;
> - }
> - else
> -
>    return wine_dbg_sprintf("[%x - %x]", caps->u.Range.UsageMin, caps->u.Range.UsageMax);
>
> +}
> -
>
> +static const char* debugstr_stringindex(struct caps *caps)
> +{
> - if (!caps->IsStringRange)
> -
>    return wine_dbg_sprintf("%i", caps->u.NotRange.StringIndex);
> - else
> -
>    return wine_dbg_sprintf("[%i - %i]", caps->u.Range.StringMin, caps->u.Range.StringMax);
>
> +}
> -
>
> +static const char* debugstr_designatorindex(struct caps *caps)
> +{
> - if (!caps->IsDesignatorRange)
> -
>    return wine_dbg_sprintf("%i", caps->u.NotRange.DesignatorIndex);
> - else
> -
>    return wine_dbg_sprintf("[%i - %i]", caps->u.Range.DesignatorMin, caps->u.Range.DesignatorMax);
>
> +}
> -
>
> +static const char* debugstr_caps(struct caps *caps)
> +{
> - if (!caps)
> -
>    return "";
> - return wine_dbg_sprintf("(UsagePage %x; LogicalMin %i; LogicalMax %i; PhysicalMin %i; PhysicalMax %i; UnitsExp %i; Units %i; BitSize %i; ReportID %i; ReportCount %i; Usage %s; StringIndex %s; DesignatorIndex %s; Delim %i;)",
> - caps->UsagePage,
> - caps->LogicalMin,
> - caps->LogicalMax,
> - caps->PhysicalMin,
> - caps->PhysicalMax,
> - caps->UnitsExp,
> - caps->Units,
> - caps->BitSize,
> - caps->ReportID,
> - caps->ReportCount,
> - debugstr_usages(caps),
> - debugstr_stringindex(caps),
> - debugstr_designatorindex(caps),
> - caps->Delim);
>
> +}
> -
>
> +static void debug_feature(struct feature *feature)
> +{
> - if (!feature)
> -
>    return;
> - TRACE("[\n\t type %s [%i]; %s; %s; %s; %s; %s; %s; %s; %s; %s\n\t %s\n]\n",
> - feature_string[feature->type],
> - feature->index,
> - (feature->isData)?"Data":"Const",
> - (feature->isArray)?"Array":"Var",
> - (feature->IsAbsolute)?"Abs":"Rel",
> - (feature->Wrap)?"Wrap":"NoWrap",
> - (feature->Linear)?"Linear":"NonLinear",
> - (feature->prefState)?"PrefStat":"NoPrefState",
> - (feature->HasNull)?"HasNull":"NoNull",
> - (feature->Volatile)?"Volatile":"NonVolatile",
> - (feature->BitField)?"BitField":"Buffered",
> - debugstr_caps(&feature->caps));
>
> +}
> -
>
> +static void debug_collection(struct collection *collection)
> +{
> - struct feature *fentry;
> - struct collection *centry;
> - if (TRACE_ON(hid))
> - {
> -
>    TRACE("{ %s[%i], %p, %i features, %i collections\n\t %s\n", collection_string[collection->type], collection->index, collection->parent, list_count(&collection->features), list_count(&collection->collections), debugstr_caps(&collection->caps));
> -
>    LIST_FOR_EACH_ENTRY(fentry, &collection->features, struct feature, col_entry)
> -
>        debug_feature(fentry);
> -
>    LIST_FOR_EACH_ENTRY(centry, &collection->collections, struct collection, entry)
> -
>        debug_collection(centry);
> -
>    TRACE("}\n");
> - }
>
> +}
> -
>
> +static void debug_print_button_cap(const CHAR * type, WINE_HID_ELEMENT *wine_element)
> +{
> - if (!wine_element->caps.button.IsRange)
> -
>    TRACE("%s Button: %x/%04x: ReportId %i, startBit %i/1\n" , type,
> -
>        wine_element->caps.button.UsagePage,
> -
>        wine_element->caps.button.u.NotRange.Usage,
> -
>        wine_element->caps.value.ReportID,
> -
>        wine_element->valueStartBit);
> - else
> -
>    TRACE("%s Button: %x/[%04x-%04x]: ReportId %i, startBit %i/%i\n" ,type,
> -
>           wine_element->caps.button.UsagePage,
> -
>           wine_element->caps.button.u.Range.UsageMin,
> -
>           wine_element->caps.button.u.Range.UsageMax,
> -
>           wine_element->caps.value.ReportID,
> -
>           wine_element->valueStartBit,
> -
>           wine_element->bitCount);
>
> +}
> -
>
> +static void debug_print_value_cap(const CHAR * type, WINE_HID_ELEMENT *wine_element)
> +{
> - TRACE("%s Value: %x/%x: ReportId %i, IsAbsolute %i, HasNull %i, "
> -
>      "Bit Size %i, ReportCount %i, UnitsExp %i, Units %i, "
> -
>      "LogicalMin %i, Logical Max %i, PhysicalMin %i, "
> -
>      "PhysicalMax %i -- StartBit %i/%i\n", type,
> -
>        wine_element->caps.value.UsagePage,
> -
>        wine_element->caps.value.u.NotRange.Usage,
> -
>        wine_element->caps.value.ReportID,
> -
>        wine_element->caps.value.IsAbsolute,
> -
>        wine_element->caps.value.HasNull,
> -
>        wine_element->caps.value.BitSize,
> -
>        wine_element->caps.value.ReportCount,
> -
>        wine_element->caps.value.UnitsExp,
> -
>        wine_element->caps.value.Units,
> -
>        wine_element->caps.value.LogicalMin,
> -
>        wine_element->caps.value.LogicalMax,
> -
>        wine_element->caps.value.PhysicalMin,
> -
>        wine_element->caps.value.PhysicalMax,
> -
>        wine_element->valueStartBit,
> -
>        wine_element->bitCount);
>
> +}
> -
>
> +static void debug_print_element(const CHAR* type, WINE_HID_ELEMENT *wine_element)
> +{
> - if (wine_element->ElementType == ButtonElement)
> -
>    debug_print_button_cap(type, wine_element);
> - else if (wine_element->ElementType == ValueElement)
> -
>    debug_print_value_cap(type, wine_element);
> - else
> -
>    TRACE("%s: UNKNOWN\n", type);
>
> +}
> -
>
> +static void debug_print_report(const char* type, WINE_HID_REPORT *report)
> +{
> - int i;
> - TRACE("%s REPORT:\n"
> -
>    "\tReportID: %i\n"
> -
>    "\tdwSize: %i\n"
> -
>    "\telementCount: %i\n",
> -
>    type,
> -
>    report->reportID,
> -
>    report->dwSize,
> -
>    report->elementCount);
> - for (i = 0; i < report->elementCount; i++)
> -
>    debug_print_element(type, &report->Elements[i]);
>
> +}
> -
>
> +static void debug_print_preparsed(WINE_HIDP_PREPARSED_DATA *data)
> +{
> - int i;
> - WINE_HID_REPORT *r;
> - if (TRACE_ON(hid))
> - {
> -
>    TRACE("PREPARSED Data:\n"
> -
>         "\tdwSize: %i\n"
> -
>         "\tCaps:\n"
> -
>         "\t\tUsage: %i\n"
> -
>         "\t\tUsagePage: %i\n"
> -
>         "\t\tInputReportByteLength: %i\n"
> -
>         "\t\tOutputReportByteLength: %i\n"
> -
>         "\t\tFeatureReportByteLength: %i\n"
> -
>         "\t\tNumberLinkCollectionNodes: %i\n"
> -
>         "\t\tNumberInputButtonCaps: %i\n"
> -
>         "\t\tNumberInputValueCaps: %i\n"
> -
>         "\t\tNumberInputDataIndices: %i\n"
> -
>         "\t\tNumberOutputButtonCaps: %i\n"
> -
>         "\t\tNumberOutputValueCaps: %i\n"
> -
>         "\t\tNumberOutputDataIndices: %i\n"
> -
>         "\t\tNumberFeatureButtonCaps: %i\n"
> -
>         "\t\tNumberFeatureValueCaps: %i\n"
> -
>         "\t\tNumberFeatureDataIndices: %i\n"
> -
>         "\tdwInputReportCount: %i\n"
> -
>         "\tdwOutputReportCount: %i\n"
> -
>         "\tdwFeatureReportCount: %i\n"
> -
>         "\tdwOutputReportOffset: %i\n"
> -
>         "\tdwFeatureReportOffset: %i\n",
> -
>    data->dwSize,
> -
>    data->caps.Usage,
> -
>    data->caps.UsagePage,
> -
>    data->caps.InputReportByteLength,
> -
>    data->caps.OutputReportByteLength,
> -
>    data->caps.FeatureReportByteLength,
> -
>    data->caps.NumberLinkCollectionNodes,
> -
>    data->caps.NumberInputButtonCaps,
> -
>    data->caps.NumberInputValueCaps,
> -
>    data->caps.NumberInputDataIndices,
> -
>    data->caps.NumberOutputButtonCaps,
> -
>    data->caps.NumberOutputValueCaps,
> -
>    data->caps.NumberOutputDataIndices,
> -
>    data->caps.NumberFeatureButtonCaps,
> -
>    data->caps.NumberFeatureValueCaps,
> -
>    data->caps.NumberFeatureDataIndices,
> -
>    data->dwInputReportCount,
> -
>    data->dwOutputReportCount,
> -
>    data->dwFeatureReportCount,
> -
>    data->dwOutputReportOffset,
> -
>    data->dwFeatureReportOffset);
> -
> -
>    r = HID_INPUT_REPORTS(data);
> -
>    for (i = 0; i < data->dwInputReportCount; i++)
> -
>    {
> -
>        debug_print_report("INPUT", r);
> -
>        r = HID_NEXT_REPORT(data, r);
> -
>    }
> -
>    r = HID_OUTPUT_REPORTS(data);
> -
>    for (i = 0; i < data->dwOutputReportCount; i++)
> -
>    {
> -
>        debug_print_report("OUTPUT", r);
> -
>        r = HID_NEXT_REPORT(data, r);
> -
>    }
> -
>    r = HID_FEATURE_REPORTS(data);
> -
>    for (i = 0; i < data->dwFeatureReportCount; i++)
> -
>    {
> -
>        debug_print_report("FEATURE", r);
> -
>        r = HID_NEXT_REPORT(data, r);
> -
>    }
> - }
>
> +}
> -
>
> +static int getValue(int bsize, int source)
> +{
> - int mask = 0xff;
> - int negative = 0x80;
> - int outofrange = 0x100;
> - int value;
> - int i;
> -
> - if (bsize == 4)
> -
>    return source;
> -
> - for (i = 1; i < bsize; i++)
> - {
> -
>    mask = (mask<<8) + 0xff;
> -
>    negative = (negative<<8);
> -
>    outofrange = (outofrange<<8);
> - }
> - value = (source&mask);
> - if (value&negative)
> -
>    value = -1 * (outofrange - value);
> - return value;
>
> +}
> -
>
> +void parse_io_feature(int bSize, int itemVal, int bTag, int *feature_index, struct feature *feature)
> +{
> - if (bSize <= 0)
> - {
> -
>    return;
> - }
> - else
> - {
> -
>    if ((itemVal & 0x01) == 0)
> -
>        feature->isData = TRUE;
> -
>    else
> -
>        feature->isData = FALSE; //Const
> -
>    if ((itemVal & 0x02) == 0)
> -
>        feature->isArray= TRUE;
> -
>    else
> -
>        feature->isArray= TRUE; //Var
> -
>    if ((itemVal & 0x04) == 0)
> -
>        feature->IsAbsolute = TRUE;
> -
>    else
> -
>        feature->IsAbsolute = FALSE; //Rel
> -
>    if ((itemVal & 0x08) == 0)
> -
>        feature->Wrap = FALSE;
> -
>    else
> -
>        feature->Wrap = TRUE;
> -
>    if ((itemVal & 0x10) == 0)
> -
>        feature->Linear = TRUE;
> -
>    else
> -
>        feature->Linear = FALSE;
> -
>    if ((itemVal & 0x20) == 0)
> -
>        feature->prefState = TRUE;
> -
>    else
> -
>        feature->prefState = FALSE;
> -
>    if ((itemVal & 0x40) == 0)
> -
>        feature->HasNull = FALSE;
> -
>    else
> -
>        feature->HasNull = TRUE;
> -
> -
>    if (bTag != 0x08)
> -
>    {
> -
>        if ((itemVal & 0x80) == 0)
> -
>            feature->Volatile = FALSE;
> -
>        else
> -
>            feature->Volatile = TRUE;
> -
>    }
> -
>    if (bSize > 1)
> -
>    {
> -
>        if ((itemVal & 0x100) == 0)
> -
>            feature->BitField = TRUE;
> -
>        else
> -
>            feature->BitField = FALSE; //Buffered Bytes
> -
>    }
> -
>    feature->index = *feature_index;
> -
>    *feature_index = *feature_index + 1;
> - }
>
> +}
> -
>
> +void parse_collection(int bSize, int itemVal, struct collection *collection)
> +{
> - if (bSize <= 0)
> -
>    return;
> - else
> -
> {
> -
>    collection->type = itemVal;
> -
> -
>    if (itemVal >= 0x07 && itemVal <= 0x7F) {
> -
>        ERR(" (Reserved %x )\n", itemVal);
> -
>    }
> -
>    else if (itemVal >= 0x80 && itemVal <= 0xFF) {
> -
>        ERR(" (Vendor Defined %x )\n", itemVal);
> -
>    }
> - }
>
> +}
> -
>
> +static void new_caps(struct caps *caps)
> +{
> - caps->IsRange = 0;
> - caps->IsStringRange = 0;
> - caps->IsDesignatorRange = 0;
> - caps->usage_count = 0;
>
> +}
> -
>
> +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;)
> - {
> -
>    BYTE b0 = descriptor[i++];
> -
>    int bSize = b0 & 0x03;
> -
>    int bType = (b0 >> 2) & 0x03;
> -
>    int bTag = (b0 >> 4) & 0x0F;
> -
> -
>    bSize = (bSize == 3) ? 4 : bSize;
> -
>    if (bType == 0x03 && bTag == 0x0F && bSize == 2 && i + 2 < length)
> -
>    {
> -
>        /* Long data items: Should be unused */
> -
>        ERR("Long Data Item, should be unused\n");

This confused me. Is this a malformed report from the HID or something
Wine doesn't yet support?
> -
>    }
> -
>    else
> -
>    {
> -
>        int bSizeActual = 0;
> -
>        int itemVal = 0;
> -
>        int j;
> -
> -
>        for (j = 0; j < bSize; j++)
> -
>        {
> -
>            if (i + j < length)
> -
>            {
> -
>                itemVal += descriptor[i + j] << (8 * j);
> -
>                bSizeActual++;
> -
>            }
> -
>        }
> -
>        TRACE(" %x[%i], type %i , tag %i, size %i, val %i\n",b0,i-1,bType, bTag, bSize, itemVal );
> -
> -
>        if (bType == 0x00)
> -
>        {
> -
>            struct feature *feature;
> -
>            switch(bTag)
> -
>            {
> -
>                case 0x08:

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?
> -
>                    feature = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*feature));
> -
>                    list_add_tail(&collection->features, &feature->col_entry);
> -
>                    list_add_tail(features, &feature->entry);
> -
>                    feature->type = HidP_Input;
> -
>                    parse_io_feature(bSize, itemVal, bTag, feature_index, feature);
> -
>                    feature->caps = *caps;
> -
>                    feature->collection = collection;
> -
>                    new_caps(caps);
> -
>                    break;
> -
>                case 0x09:
> -
>                    feature = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*feature));
> -
>                    list_add_tail(&collection->features, &feature->col_entry);
> -
>                    list_add_tail(features, &feature->entry);
> -
>                    feature->type = HidP_Output;
> -
>                    parse_io_feature(bSize, itemVal, bTag, feature_index, feature);
> -
>                    feature->caps = *caps;
> -
>                    feature->collection = collection;
> -
>                    new_caps(caps);
> -
>                    break;
> -
>                case 0x0B:
> -
>                    feature = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*feature));
> -
>                    list_add_tail(&collection->features, &feature->col_entry);
> -
>                    list_add_tail(features, &feature->entry);
> -
>                    feature->type = HidP_Feature;
> -
>                    parse_io_feature(bSize, itemVal, bTag, feature_index, feature);
> -
>                    feature->caps = *caps;
> -
>                    feature->collection = collection;
> -
>                    new_caps(caps);
> -
>                    break;
> -
>                case 0x0A:

Here, the cases are in a different order. That wouldn't matter with
symbols, but it struck me with the raw numbers.
> -
>                {
> -
>                    struct collection *subcollection = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(struct collection));
> -
>                    list_add_tail(&collection->collections, &subcollection->entry);
> -
>                    subcollection->parent = collection;
> -
>                    /* Only set our collection once...
> -
>                       We do not properly handle composite devices yet. */
> -
>                    if (*collection_index == 0)
> -
>                        collection->caps = *caps;
> -
>                    subcollection->caps = *caps;
> -
>                    subcollection->index = *collection_index;
> -
>                    *collection_index = *collection_index + 1;
> -
>                    list_init(&subcollection->features);
> -
>                    list_init(&subcollection->collections);
> -
>                    new_caps(caps);
> -
> -
>                    parse_collection(bSize, itemVal, subcollection);
> -
> -
>                    i = parse_descriptor(descriptor, i+1, length, feature_index, collection_index, subcollection, caps, features);
> -
>                    continue;
> -
>                }
> -
>                case 0x0C:
> -
>                    return i;
> -
>                default:
> -
>                    ERR("Unknown Tag\n");

Would it be useful to print the tag value here?
> -
>            }
> -
>        }
> -
>        else if (bType == 0x01)
> -
>        {
> -
>            switch(bTag)
> -
>            {
> -
>                case 0x00:
> -
>                    caps->UsagePage = getValue(bSize, itemVal);
> -
>                    break;
> -
>                case 0x01:
> -
>                    caps->LogicalMin = getValue(bSize, itemVal);
> -
>                    break;
> -
>                case 0x02:
> -
>                    caps->LogicalMax = getValue(bSize, itemVal);
> -
>                    break;
> -
>                case 0x03:
> -
>                    caps->PhysicalMin = getValue(bSize, itemVal);
> -
>                    break;
> -
>                case 0x04:
> -
>                    caps->PhysicalMax = getValue(bSize, itemVal);
> -
>                    break;
> -
>                case 0x05:
> -
>                    caps->UnitsExp = getValue(bSize, itemVal);
> -
>                    break;
> -
>                case 0x06:
> -
>                    caps->Units = getValue(bSize, itemVal);
> -
>                    break;
> -
>                case 0x07:
> -
>                    caps->BitSize = getValue(bSize, itemVal);
> -
>                    break;
> -
>                case 0x08:
> -
>                    caps->ReportID = getValue(bSize, itemVal);
> -
>                    break;
> -
>                case 0x09:
> -
>                    caps->ReportCount = getValue(bSize, itemVal);
> -
>                    break;
> -
>                case 0x0A:
> -
>                    ERR("Push\n");
> -
>                    break;
> -
>                case 0x0B:
> -
>                    ERR("Pop\n");
> -
>                    break;

Are these invalid messages from the HID device, or things wine doesn't
yet support? Should they be FIXMEs?
> -
>                default:
> -
>                    ERR("Unknown (bTag: %x, bType: %x)\n", bTag, bType);
> -
>            }
> -
>        }
> -
>        else if (bType == 0x02)
> -
>        {
> -
>            switch(bTag)
> -
>            {
> -
>                case 0x00:
> -
>                    if (caps->usage_count >= USAGE_MAX)
> -
>                        FIXME("More than %i individual usages defined\n",USAGE_MAX);
> -
>                    else
> -
>                    {
> -
>                        caps->u.NotRange.Usage[caps->usage_count++] = getValue(bSize, itemVal);
> -
>                        caps->IsRange = FALSE;
> -
>                    }
> -
>                    break;
> -
>                case 0x01:
> -
>                    caps->usage_count = 1;
> -
>                    caps->u.Range.UsageMin = getValue(bSize, itemVal);
> -
>                    caps->IsRange = TRUE;
> -
>                    break;
> -
>                case 0x02:
> -
>                    caps->usage_count = 1;
> -
>                    caps->u.Range.UsageMax = getValue(bSize, itemVal);
> -
>                    caps->IsRange = TRUE;
> -
>                    break;
> -
>                case 0x03:
> -
>                    caps->u.NotRange.DesignatorIndex = getValue(bSize, itemVal);
> -
>                    caps->IsDesignatorRange = FALSE;
> -
>                    break;
> -
>                case 0x04:
> -
>                    caps->u.Range.DesignatorMin = getValue(bSize, itemVal);
> -
>                    caps->IsDesignatorRange = TRUE;
> -
>                    break;
> -
>                case 0x05:
> -
>                    caps->u.Range.DesignatorMax = getValue(bSize, itemVal);
> -
>                    caps->IsDesignatorRange = TRUE;
> -
>                    break;
> -
>                case 0x07:
> -
>                    caps->u.NotRange.StringIndex = getValue(bSize, itemVal);
> -
>                    caps->IsStringRange = FALSE;
> -
>                    break;
> -
>                case 0x08:
> -
>                    caps->u.Range.StringMin = getValue(bSize, itemVal);
> -
>                    caps->IsStringRange = TRUE;
> -
>                    break;
> -
>                case 0x09:
> -
>                    caps->u.Range.StringMax = getValue(bSize, itemVal);
> -
>                    caps->IsStringRange = TRUE;
> -
>                    break;
> -
>                case 0x0A:
> -
>                    caps->Delim = getValue(bSize, itemVal);
> -
>                    break;
> -
>                default:
> -
>                    ERR("Unknown (bTag: %x, bType: %x)\n", bTag, bType);
> -
>            }
> -
>        }
> -
>        else
> -
>            ERR("Unknown (bTag: %x, bType: %x)\n", bTag, bType);
> -
> -
>        i += bSize;
> -
>    }
> - }
> - return i;
>
> +}
> -
>
> +static inline void new_report(WINE_HID_REPORT *wine_report, struct feature* feature)
> +{
> - wine_report->reportID = feature->caps.ReportID;
> - wine_report->dwSize = sizeof(*wine_report) - sizeof(WINE_HID_ELEMENT);
> - wine_report->elementCount = 0;
>
> +}
> -
>
> +static void build_elements(WINE_HID_REPORT *wine_report, struct feature* feature, DWORD *bitOffset)
> +{
> - int i;
> -
> - if (!feature->isData)
> - {
> -
>    *bitOffset = *bitOffset + (feature->caps.BitSize * feature->caps.ReportCount);
> -
>    return;
> - }
> -
> - for (i = 0; i < feature->caps.usage_count; i++)
> - {
> -
>    WINE_HID_ELEMENT *wine_element = &wine_report->Elements[wine_report->elementCount];
> -
> -
>    wine_element->valueStartBit = *bitOffset;
> -
>    if (feature->caps.UsagePage == HID_USAGE_PAGE_BUTTON)
> -
>    {
> -
>        wine_element->ElementType = ButtonElement;
> -
>        wine_element->caps.button.UsagePage = feature->caps.UsagePage;
> -
>        wine_element->caps.button.ReportID = feature->caps.ReportID;
> -
>        wine_element->caps.button.BitField = feature->BitField;
> -
>        wine_element->caps.button.IsRange = feature->caps.IsRange;
> -
>        wine_element->caps.button.IsStringRange = feature->caps.IsStringRange;
> -
>        wine_element->caps.button.IsDesignatorRange = feature->caps.IsDesignatorRange;
> -
>        wine_element->caps.button.IsAbsolute = feature->IsAbsolute;
> -
>        if (wine_element->caps.button.IsRange)
> -
>        {
> -
>            wine_element->bitCount = (feature->caps.u.Range.UsageMax - feature->caps.u.Range.UsageMin) + 1;
> -
>            *bitOffset = *bitOffset + wine_element->bitCount;
> -
>            wine_element->caps.button.u.Range.UsageMin = feature->caps.u.Range.UsageMin;
> -
>            wine_element->caps.button.u.Range.UsageMax = feature->caps.u.Range.UsageMax;
> -
>            wine_element->caps.button.u.Range.StringMin = feature->caps.u.Range.StringMin;
> -
>            wine_element->caps.button.u.Range.StringMax = feature->caps.u.Range.StringMax;
> -
>            wine_element->caps.button.u.Range.DesignatorMin = feature->caps.u.Range.DesignatorMin;
> -
>            wine_element->caps.button.u.Range.DesignatorMax = feature->caps.u.Range.DesignatorMax;
> -
>        }
> -
>        else
> -
>        {
> -
>            *bitOffset = *bitOffset + 1;
> -
>            wine_element->bitCount = 1;
> -
>            wine_element->caps.button.u.NotRange.Usage = feature->caps.u.NotRange.Usage[i];
> -
>            wine_element->caps.button.u.NotRange.StringIndex = feature->caps.u.NotRange.StringIndex;
> -
>            wine_element->caps.button.u.NotRange.DesignatorIndex = feature->caps.u.NotRange.DesignatorIndex;
> -
>        }
> -
>    }
> -
>    else
> -
>    {
> -
>        wine_element->ElementType = ValueElement;
> -
>        wine_element->caps.value.UsagePage = feature->caps.UsagePage;
> -
>        wine_element->caps.value.ReportID = feature->caps.ReportID;
> -
>        wine_element->caps.value.BitField = feature->BitField;
> -
>        wine_element->caps.value.IsRange = feature->caps.IsRange;
> -
>        wine_element->caps.value.IsStringRange = feature->caps.IsStringRange;
> -
>        wine_element->caps.value.IsDesignatorRange = feature->caps.IsDesignatorRange;
> -
>        wine_element->caps.value.IsAbsolute = feature->IsAbsolute;
> -
>        wine_element->caps.value.HasNull = feature->HasNull;
> -
>        wine_element->caps.value.BitSize = feature->caps.BitSize;
> -
>        if (feature->caps.usage_count > 1)
> -
>            wine_element->caps.value.ReportCount = 1;
> -
>        else
> -
>            wine_element->caps.value.ReportCount = feature->caps.ReportCount;
> -
>        wine_element->bitCount = (feature->caps.BitSize * wine_element->caps.value.ReportCount);
> -
>        *bitOffset = *bitOffset + wine_element->bitCount;
> -
>        wine_element->caps.value.UnitsExp = feature->caps.UnitsExp;
> -
>        wine_element->caps.value.Units = feature->caps.Units;
> -
>        wine_element->caps.value.LogicalMin = feature->caps.LogicalMin;
> -
>        wine_element->caps.value.LogicalMax = feature->caps.LogicalMax;
> -
>        wine_element->caps.value.PhysicalMin = feature->caps.PhysicalMin;
> -
>        wine_element->caps.value.PhysicalMax = feature->caps.PhysicalMax;
> -
>        if (wine_element->caps.value.IsRange)
> -
>        {
> -
>            wine_element->caps.value.u.Range.UsageMin = feature->caps.u.Range.UsageMin;
> -
>            wine_element->caps.value.u.Range.UsageMax = feature->caps.u.Range.UsageMax;
> -
>            wine_element->caps.value.u.Range.StringMin = feature->caps.u.Range.StringMin;
> -
>            wine_element->caps.value.u.Range.StringMax = feature->caps.u.Range.StringMax;
> -
>            wine_element->caps.value.u.Range.DesignatorMin = feature->caps.u.Range.DesignatorMin;
> -
>            wine_element->caps.value.u.Range.DesignatorMax = feature->caps.u.Range.DesignatorMax;
> -
>        }
> -
>        else
> -
>        {
> -
>            wine_element->caps.value.u.NotRange.Usage = feature->caps.u.NotRange.Usage[i];
> -
>            wine_element->caps.value.u.NotRange.StringIndex = feature->caps.u.NotRange.StringIndex;
> -
>            wine_element->caps.value.u.NotRange.DesignatorIndex = feature->caps.u.NotRange.DesignatorIndex;
> -
>        }
> -
>    }
> -
> -
>    wine_report->elementCount++;
> - }
>
> +}
> -
>
> +static void count_elements(struct feature* feature, USHORT *buttons, USHORT *values)
> +{
> - if (feature->caps.UsagePage == HID_USAGE_PAGE_BUTTON)
> - {
> -
>    if (feature->caps.IsRange)
> -
>        *buttons = *buttons + 1;
> -
>    else
> -
>        *buttons = *buttons + feature->caps.usage_count;
> - }
> - else
> - {
> -
>    if (feature->caps.IsRange)
> -
>        *values = *values + 1;
> -
>    else
> -
>        *values = *values + feature->caps.usage_count;
> - }
>
> +}
> -
>
> +WINE_HIDP_PREPARSED_DATA* build_PreparseData(
> -
>                   struct feature **features, int feature_count,
> -
>                   struct feature **input_features, int i_count,
> -
>                   struct feature **output_features, int o_count,
> -
>                   struct feature **feature_features, int f_count,
> -
>                   struct collection *base_collection)
>
> +{
> - WINE_HIDP_PREPARSED_DATA *data;
> - WINE_HID_REPORT *wine_report = NULL;
> - DWORD bitOffset, bitLength;
> - int report_count = 1;
> - int i;
> - int element_count;
> - int size = 0;
> -
> - if (features[0]->caps.ReportID != 0)
> - {
> -
>    int *report_ids;
> -
>    int cnt = max(i_count, o_count);
> -
>    cnt = max(cnt, f_count);
> -
>    report_ids = HeapAlloc(GetProcessHeap(), 0 , sizeof(*report_ids) * cnt);
> -
> -
>    if (i_count)
> -
>    {
> -
>        report_ids[0] = input_features[0]->caps.ReportID;
> -
>        for (i = 1; i < i_count; i++)
> -
>        {
> -
>            int j;
> -
>            int found = FALSE;
> -
>            for (j = 0; !found && j < i_count; j++)
> -
>            {
> -
>                if (report_ids[j] == input_features[i]->caps.ReportID)
> -
>                    found = TRUE;
> -
>            }
> -
>            if (!found)
> -
>            {
> -
>                report_ids[report_count] = input_features[i]->caps.ReportID;
> -
>                report_count++;
> -
>            }
> -
>        }
> -
>    }
> -
>    if (o_count)
> -
>    {
> -
>        report_count++;
> -
>        report_ids[0] = output_features[0]->caps.ReportID;
> -
>        for (i = 1; i < o_count; i++)
> -
>        {
> -
>            int j;
> -
>            int found = FALSE;
> -
>            for (j = 0; !found && j < o_count; j++)
> -
>            {
> -
>                if (report_ids[j] == output_features[i]->caps.ReportID)
> -
>                    found = TRUE;
> -
>            }
> -
>            if (!found)
> -
>            {
> -
>                report_ids[report_count] = output_features[i]->caps.ReportID;
> -
>                report_count++;
> -
>            }
> -
>        }
> -
>    }
> -
>    if (f_count)
> -
>    {
> -
>        report_count++;
> -
>        report_ids[0] = feature_features[0]->caps.ReportID;
> -
>        for (i = 1; i < f_count; i++)
> -
>        {
> -
>            int j;
> -
>            int found = FALSE;
> -
>            for (j = 0; !found && j < f_count; j++)
> -
>            {
> -
>                if (report_ids[j] == feature_features[i]->caps.ReportID)
> -
>                    found = TRUE;
> -
>            }
> -
>            if (!found)
> -
>            {
> -
>                report_ids[report_count] = feature_features[i]->caps.ReportID;
> -
>                report_count++;
> -
>            }
> -
>        }
> -
>    }
> -
>    HeapFree(GetProcessHeap(), 0, report_ids);
> - }
> - else
> - {
> -
>    if (o_count) report_count++;
> -
>    if (f_count) report_count++;
> - }
> -
> - element_count = 0;
> - for (i = 0; i < feature_count; i++)
> -
>    element_count += features[i]->caps.usage_count;
> -
> - size = sizeof(WINE_HIDP_PREPARSED_DATA) +
> -
>        (element_count * sizeof(WINE_HID_ELEMENT)) +
> -
>        (report_count * sizeof(WINE_HID_REPORT));
> -
> - TRACE("%i reports %i elements -> size %i\n",report_count, element_count, size);
> -
> - data = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, size);
> - data->magic = HID_MAGIC;
> - data->dwSize = size;
> - data->caps.Usage = base_collection->caps.u.NotRange.Usage[0];
> - data->caps.UsagePage = base_collection->caps.UsagePage;
> -
> - wine_report = data->InputReports;
> - if (i_count)
> - {
> -
>    bitLength = 0;
> -
>    new_report(wine_report, input_features[0]);
> -
>    data->dwInputReportCount ++;
> -
> -
>    if (input_features[0]->caps.ReportID != 0)
> -
>        bitOffset = 8;
> -
>    else
> -
>        bitOffset = 0;
> -
> -
>    for (i = 0; i < i_count; i++)
> -
>    {
> -
>        if (input_features[i]->caps.ReportID != wine_report->reportID)
> -
>        {
> -
>            wine_report->dwSize += (sizeof(WINE_HID_ELEMENT) * wine_report->elementCount);
> -
>            wine_report = (WINE_HID_REPORT*)(((BYTE*)wine_report)+wine_report->dwSize);
> -
>            new_report(wine_report, input_features[i]);
> -
>            data->dwInputReportCount ++;
> -
>            bitLength = max(bitOffset, bitLength);
> -
>            if (input_features[i]->caps.ReportID != 0)
> -
>                bitOffset = 8;
> -
>            else
> -
>                bitOffset = 0;
> -
>        }
> -
>        build_elements(wine_report, input_features[i], &bitOffset);
> -
>        count_elements(input_features[i], &data->caps.NumberInputButtonCaps,
> -
>            &data->caps.NumberInputValueCaps);
> -
>    }
> -
>    wine_report->dwSize += (sizeof(WINE_HID_ELEMENT) * wine_report->elementCount);
> -
>    bitLength = max(bitOffset, bitLength);
> -
>    data->caps.InputReportByteLength = ((bitLength + 7) & ~7)/8;
> - }
> -
> - if (o_count)
> - {
> -
>    bitLength = 0;
> -
>    wine_report = (WINE_HID_REPORT*)(((BYTE*)wine_report)+wine_report->dwSize);
> -
>    data->dwOutputReportOffset = (BYTE*)wine_report - (BYTE*)data->InputReports;
> -
>    new_report(wine_report, output_features[0]);
> -
>    data->dwOutputReportCount ++;
> -
>    if (output_features[0]->caps.ReportID != 0)
> -
>        bitOffset = 8;
> -
>    else
> -
>        bitOffset = 0;
> -
> -
>    for (i = 0; i < o_count; i++)
> -
>    {
> -
>        if (output_features[i]->caps.ReportID != wine_report->reportID)
> -
>        {
> -
>            wine_report->dwSize += (sizeof(WINE_HID_ELEMENT) * wine_report->elementCount);
> -
>            wine_report = (WINE_HID_REPORT*)(((BYTE*)wine_report)+wine_report->dwSize);
> -
>            new_report(wine_report, output_features[i]);
> -
>            data->dwOutputReportCount ++;
> -
>            bitLength = max(bitOffset, bitLength);
> -
>            if (output_features[0]->caps.ReportID != 0)
> -
>                bitOffset = 8;
> -
>            else
> -
>                bitOffset = 0;
> -
>        }
> -
>        build_elements(wine_report, output_features[i], &bitOffset);
> -
>        count_elements(output_features[i], &data->caps.NumberOutputButtonCaps,
> -
>            &data->caps.NumberOutputValueCaps);
> -
>    }
> -
>    wine_report->dwSize += (sizeof(WINE_HID_ELEMENT) * wine_report->elementCount);
> -
>    bitLength = max(bitOffset, bitLength);
> -
>    data->caps.OutputReportByteLength = ((bitLength + 7) & ~7)/8;
> - }
> -
> - if (f_count)
> - {
> -
>    bitLength = 0;
> -
>    wine_report = (WINE_HID_REPORT*)(((BYTE*)wine_report)+wine_report->dwSize);
> -
>    data->dwFeatureReportOffset = (BYTE*)wine_report - (BYTE*)data->InputReports;
> -
>    new_report(wine_report, feature_features[0]);
> -
>    data->dwFeatureReportCount ++;
> -
>    if (feature_features[0]->caps.ReportID != 0)
> -
>        bitOffset = 8;
> -
>    else
> -
>        bitOffset = 0;
> -
> -
>    for (i = 0; i < f_count; i++)
> -
>    {
> -
>        if (feature_features[i]->caps.ReportID != wine_report->reportID)
> -
>        {
> -
>            wine_report->dwSize += (sizeof(WINE_HID_ELEMENT) * wine_report->elementCount);
> -
>            wine_report = (WINE_HID_REPORT*)((BYTE*)wine_report)+wine_report->dwSize;
> -
>            new_report(wine_report, feature_features[i]);
> -
>            data->dwFeatureReportCount ++;
> -
>            bitLength = max(bitOffset, bitLength);
> -
>            if (feature_features[0]->caps.ReportID != 0)
> -
>                bitOffset = 8;
> -
>            else
> -
>                bitOffset = 0;
> -
>        }
> -
>        build_elements(wine_report, feature_features[i], &bitOffset);
> -
>        count_elements(feature_features[i], &data->caps.NumberFeatureButtonCaps,
> -
>            &data->caps.NumberFeatureValueCaps);
> -
>    }
> -
>    bitLength = max(bitOffset, bitLength);
> -
>    data->caps.FeatureReportByteLength = ((bitLength + 7) & ~7)/8;
> - }
> -
> - return data;
>
> +}
> -
>
> +static void free_collection(struct collection *collection)
> +{
> - struct feature *fentry, *fnext;
> - struct collection *centry, *cnext;
> - LIST_FOR_EACH_ENTRY_SAFE(centry, cnext, &collection->collections, struct collection, entry)
> - {
> -
>    list_remove(&centry->entry);
> -
>    free_collection(centry);
> - }
> - LIST_FOR_EACH_ENTRY_SAFE(fentry, fnext, &collection->features, struct feature, col_entry)
> - {
> -
>    list_remove(&fentry->col_entry);
> -
>    HeapFree(GetProcessHeap(), 0, fentry);
> - }
> - HeapFree(GetProcessHeap(), 0, collection);
>
> +}
> -
>
> +static int compare_reports(const void *a, const void* b)
> +{
> - struct feature *f1 = *(struct feature **)a;
> - struct feature *f2 = *(struct feature **)b;
> - int c = (f1->caps.ReportID - f2->caps.ReportID);
> - if (c) return c;
> - return (f1->index - f2->index);
>
> +}
> -
>
> +WINE_HIDP_PREPARSED_DATA* ParseDescriptor(BYTE *descriptor, INT length)
> +{
> - WINE_HIDP_PREPARSED_DATA *data = NULL;
> - struct collection *base;
> - struct caps caps;
> -
> - struct list features;
> -
> - int feature_count = 0;
> - int cidx;
> -
> - if (TRACE_ON(hid))
> - {
> -
>    TRACE("Descriptor[%i]: ", length);
> -
>    for (cidx = 0; cidx < length; cidx++)
> -
>        TRACE("%x ",descriptor[cidx]);
> -
>    TRACE("\n");
> - }
> -
> - list_init(&features);
> -
> - base = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*base));
> - base->index = 1;
> - list_init(&base->features);
> - list_init(&base->collections);
> - memset(&caps, 0, sizeof(caps));
> -
> - cidx = 0;
> - parse_descriptor(descriptor, 0, length, &feature_count, &cidx, base, &caps, &features);
> -
> - debug_collection(base);
> -
> - cidx = 2;
> - if (feature_count)
> - {
> -
>    struct feature *entry;
> -
>    struct feature** sorted_features;
> -
>    struct feature** input_features;
> -
>    struct feature** output_features;
> -
>    struct feature** feature_features;
> -
>    int i_count, o_count, f_count;
> -
>    int i;
> -
> -
>    i_count = o_count = f_count = 0;
> -
> -
>    sorted_features = HeapAlloc(GetProcessHeap(), 0, sizeof(*sorted_features) * feature_count);
> -
>    input_features = HeapAlloc(GetProcessHeap(), 0, sizeof(*input_features) * feature_count);
> -
>    output_features = HeapAlloc(GetProcessHeap(), 0, sizeof(*output_features) * feature_count);
> -
>    feature_features = HeapAlloc(GetProcessHeap(), 0, sizeof(*feature_features) * feature_count);
> -
> -
>    i = 0;
> -
>    LIST_FOR_EACH_ENTRY(entry, &features, struct feature, entry)
> -
>        sorted_features[i++] = entry;
> -
> -
>    /* Sort features base on report if there are multiple reports */
> -
>    if (sorted_features[0]->caps.ReportID != 0)
> -
>        qsort(sorted_features, feature_count, sizeof(struct feature*), compare_reports);
> -
> -
>    for (i = 0; i < feature_count; i++)
> -
>    {
> -
>        switch (sorted_features[i]->type)
> -
>        {
> -
>            case HidP_Input:
> -
>                input_features[i_count] = sorted_features[i];
> -
>                i_count++;
> -
>                break;
> -
>            case HidP_Output:
> -
>                output_features[o_count] = sorted_features[i];
> -
>                o_count++;
> -
>                break;
> -
>            case HidP_Feature:
> -
>                feature_features[f_count] = sorted_features[i];
> -
>                f_count++;
> -
>                break;
> -
>            default:
> -
>                ERR("Unknown type %i\n",sorted_features[i]->type);
> -
>        }
> -
>    }
> -
> -
>    if (TRACE_ON(hid))
> -
>    {
> -
>        TRACE("DUMP FEATURES:\n");
> -
>        TRACE("----INPUT----\n");
> -
>        for (cidx = 0; cidx < i_count; cidx++)
> -
>            debug_feature(input_features[cidx]);
> -
>        TRACE("----OUTPUT----\n");
> -
>        for (cidx = 0; cidx < o_count; cidx++)
> -
>            debug_feature(output_features[cidx]);
> -
>        TRACE("----FEATURE----\n");
> -
>        for (cidx = 0; cidx < f_count; cidx++)
> -
>            debug_feature(feature_features[cidx]);
> -
>    }
> -
> -
>    data = build_PreparseData(sorted_features, feature_count, input_features, i_count, output_features, o_count, feature_features, f_count, base);
> -
> -
>    debug_print_preparsed(data);
> -
> -
>    HeapFree(GetProcessHeap(), 0, sorted_features);
> -
>    HeapFree(GetProcessHeap(), 0, input_features);
> -
>    HeapFree(GetProcessHeap(), 0, output_features);
> -
>    HeapFree(GetProcessHeap(), 0, feature_features);
> - }
> -
> - free_collection(base);
> - /* We do not have to free the list as free_collection does all the work */
> -
> - return data;
>
> +}
> diff --git a/dlls/hidclass.sys/hid.h b/dlls/hidclass.sys/hid.h
> index 8f9e56f..c8cee69 100644
> --- a/dlls/hidclass.sys/hid.h
> +++ b/dlls/hidclass.sys/hid.h
> @@ -81,3 +81,6 @@ void HID_DeleteDevice(HID_MINIDRIVER_REGISTRATION *driver, DEVICE_OBJECT *device
>  /* Pseudo-Plug and Play support*/
>  NTSTATUS WINAPI PNP_AddDevice(DRIVER_OBJECT *driver, void* native) DECLSPEC_HIDDEN;
>  void PNP_CleanupPNP() DECLSPEC_HIDDEN;
> -
>
> +/* Parsing HID Report Descriptors into preparsed data */
> +WINE_HIDP_PREPARSED_DATA* ParseDescriptor(BYTE *descriptor, INT length) DECLSPEC_HIDDEN;
> diff --git a/dlls/hidclass.sys/pnp.c b/dlls/hidclass.sys/pnp.c
> index ca85cb7..5694b8a 100644
> --- a/dlls/hidclass.sys/pnp.c
> +++ b/dlls/hidclass.sys/pnp.c
> @@ -53,6 +53,9 @@ NTSTATUS WINAPI PNP_AddDevice(DRIVER_OBJECT *driver, void *native)
>      DWORD index = HID_STRING_ID_ISERIALNUMBER;
>      NATIVE_DEVICE *tracked_device, *ptr;
>      INT interface_index = 1;
> -
>
> HID_DESCRIPTOR descriptor;
> -
>
> BYTE *reportDescriptor;
> -
>
> INT i;
>
> static const WCHAR ig_fmtW[] = {'I','G','_','%','i',0};
> static const WCHAR im_fmtW[] = {'I','M','_','%','i',0};
>
> @@ -104,6 +107,40 @@ NTSTATUS WINAPI PNP_AddDevice(DRIVER_OBJECT *driver, void *native)
>  list_add_tail(&tracked_devices, &tracked_device->entry);
>
> - status = call_minidriver(IOCTL_HID_GET_DEVICE_DESCRIPTOR, device, NULL, 0,
> -
>    &descriptor, sizeof(descriptor));
> - if (status != STATUS_SUCCESS)
> - {
> -
>    ERR("Cannot get Device Descriptor(%x)\n",status);
> -
>    HID_DeleteDevice(&minidriver->minidriver, device);
> -
>    return status;
> - }
> - for (i = 0; i < descriptor.bNumDescriptors; i++)
> -
>    if (descriptor.DescriptorList[i].bReportType == HID_REPORT_DESCRIPTOR_TYPE)
> -
>        break;
> -
> - if (i >= descriptor.bNumDescriptors)
> - {
> -
>    ERR("No Report Descriptor found in reply\n");
> -
>    HID_DeleteDevice(&minidriver->minidriver, device);
> -
>    return status;
> - }
> -
> - reportDescriptor = HeapAlloc(GetProcessHeap(), 0, descriptor.DescriptorList[i].wReportLength);
> - status = call_minidriver(IOCTL_HID_GET_REPORT_DESCRIPTOR, device, NULL, 0,
> -
>    reportDescriptor, descriptor.DescriptorList[i].wReportLength);
> - if (status != STATUS_SUCCESS)
> - {
> -
>    ERR("Cannot get Report Descriptor(%x)\n",status);
> -
>    HID_DeleteDevice(&minidriver->minidriver, device);
> -
>    HeapFree(GetProcessHeap(), 0, reportDescriptor);
> -
>    return status;
> - }
> -
> - ext->preparseData = ParseDescriptor(reportDescriptor, descriptor.DescriptorList[0].wReportLength);
> - ext->information.DescriptorSize = ext->preparseData->dwSize;
> - HeapFree(GetProcessHeap(), 0, reportDescriptor);
> - status = call_minidriver(IOCTL_HID_GET_STRING, device,
>       (void*)index, sizeof(DWORD), serial, sizeof(serial));
>

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

[Aric Stewart]

Thanks for the review:

On 9/2/15 1:53 PM, Andrew Eikum wrote:
> A couple small notes inline below.
> > -
> >    bSize = (bSize == 3) ? 4 : bSize;
> > -
> >    if (bType == 0x03 && bTag == 0x0F && bSize == 2 && i + 2 < length)
> > -
> >    {
> > -
> >        /* Long data items: Should be unused */
> > -
> >        ERR("Long Data Item, should be unused\n");
>
> This confused me. Is this a malformed report from the HID or something
> Wine doesn't yet support?

It is in the spec but my reading I found statements that it was an option that was unused. So I wanted to highlight it if it was found but did not need to handle it.
> > -
> >        if (bType == 0x00)
> > -
> >        {
> > -
> >            struct feature *feature;
> > -
> >            switch(bTag)
> > -
> >            {
> > -
> >                case 0x08:
>
> 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. USB Report Descriptors are almost always written in a format like:

0x05, 0x01,                    // USAGE_PAGE (Generic Desktop)
    0x09, 0x05,                    // USAGE (Game Pad)
    0xa1, 0x01,                    // COLLECTION (Application)
    0xa1, 0x00,                    //   COLLECTION (Physical)
    0x85, 0x01, 		  //     REPORT_ID (1)
  ...
Where the hex values are the actual values in the report and the comments have the human readable meanings.

Other comments are being integrated and the patch resent

thanks again!
-aric

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

[Henri Verbeet]

On 3 September 2015 at 14:09, Aric Stewart  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.