Re: [PATCH 2/3] xinput: Added the function XInputGetState with test cases

[Vitaliy Margolen]

Andrew Fenn wrote:
> This patch adds the function XInputGetState which is for checking if
> any joysticks are plugged in. This always returns no joysticks.
>
> ------------------------------------------------------------------------
>
> +static void test_no_controller(void) {
> +
> +    XINPUT_STATE controllerState;
> +    DWORD controllerNum;
> +    DWORD result;
> +
> +    for(controllerNum=0; controllerNum < 4; controllerNum++) {
> +        ZeroMemory(&controllerState, sizeof(XINPUT_STATE));
> +
> +        result = XInputGetState(controllerNum, &controllerState);
> +        ok(result != ERROR_BAD_ARGUMENTS, "XInputGetState failed with (%d)\n", result);
> +
> +        if (ERROR_DEVICE_NOT_CONNECTED == result) {
> +            skip("Controller %d is not connected\n", controllerNum);
> +        } else {

and
> +    if (hXinput)
> +    {
> +        XInputGetState = (void*)GetProcAddress(hXinput, "XInputGetState");
> +        test_no_controller();
> +    } else {
> +        skip("Could not load xinput1_3.dll\n");
> +    }

Using 2 separate stiles for curly brackets. Please put the opening and
closing brackets on separate lines. It makes it so much easer to read. Ex:

if (something)
{
  /* Doing something */
}
else
{
  /* Doing something else */
}

In xinput.h you adding lots of defines you not using. And you just threw all
of them together in one big pile. Please add them when you need them. Or at
least put them into proper groups.
> +    if (dwUserIndex < 4) {
> +        return ERROR_DEVICE_NOT_CONNECTED;
> +        /* If controller exists then return ERROR_SUCCESS */
> +    }
> Why 4? Why not 40? Have you run your tests on windows?
> -static void test_no_controller(void) {
> +
> +
> +static void test_get_state(void) {
> You should call it the right way from the start.

Vitaliy.