(resend 2)[2/5]hidclass.sys: Implement a report ring buffer

Henri Verbeet hverbeet at gmail.com
Mon Jul 13 07:54:29 CDT 2015


On 13 July 2015 at 14:13, Aric Stewart <aric at codeweavers.com> wrote:
> Yeah, the trick partly is there is a lot of ground work to do. Yes, most of these patches are not used until later patches. But I need to build that ground work so that the later patches work and it is not all introduced in one mammoth patch.
>
Yeah, it can be a bit tricky to split things like that in a useful
way. At the same time, it's important to be able to see how things are
called by other code.

> 5 was picked arbitrarily.  I guess 2 or any other number would be just fine.
>
It mostly depends on how many readers/writers you expect in the common
case, but typically you'd use some power of two for these kinds of
dynamically growing arrays. Of course if you already know in advance
that there will only ever be a fixed number you can simplify things.

>>> +VOID RingBuffer_Destroy(struct __ReportRingBuffer *ring)
>> Why use "VOID"? Are you concerned "void" may not be available?
>>
>
> No, this is mostly because I did try to standardize my code a bit to use the VOID identifier instead of the void identifier, so clearly made the wrong choice there.
>
>>> +    if (size < 2 || size > 256 || size == ring->size)
>>> +        return;
>> What's special about 2 and 256? And if this does nothing, why would
>> you ever call the function like that?
>>
>
> Those are limits on the ring buffer stated in MSDN for HID ring buffers.
> IOCTL_SET_NUM_DEVICE_INPUT_BUFFERS sets that value.
>
Right, you'll probably to add a #define or something for those then
instead of just having magic constants. It's hard to judge without the
code using this, but it may be the case that this is better enforced
in the caller. Depending on how RingBuffer_SetSize() is going to be
used, it may also be the case that you could just as well just destroy
the entire ringbuffer and recreate it.



More information about the wine-devel mailing list