Misc cleanup for `src/joystick/hidapi/SDL_report_descriptor.c` and new test program (#15959)

* testevdev: Always use decimal for HID report descriptors size

The size of the HID report descriptors in the SDL_COMPILE_TIME_ASSERT
checks is specified sometimes in decimal and sometimes in octal.

Always used decimal for consistency.

Signed-off-by: Antonio Ospite <antonio.ospite@collabora.com>

* SDL_report_descriptor: Add missing include guards in header file

Signed-off-by: Antonio Ospite <antonio.ospite@collabora.com>

* SDL_report_descriptor: Fix some ignored return values

Fix some ignored return values resulting in incomplete error checking.

For instance:

  - ParseMainItem() was always returning true because the return value of
    AddInputFields() was not handled.

  - ParseLocalItem() was always returning true because the return value of
    AddUsage() was not handled.

Signed-off-by: Antonio Ospite <antonio.ospite@collabora.com>

* test: Add new test program for SDL_report_descriptor code

Add support for testing the SDL_report_descriptor code by adding a new
executable to make it easier to experiment with HID report descriptor
parsing.

The main() function which originally was in
src/joystick/hidapi/SDL_report_descriptor.c is moved to a separate file
named test/testdescriptor.c in order to simplify dealing with multiple
build systems.

For instance if the main() was in
src/joystick/hidapi/SDL_report_descriptor.c the code would have to
account for the cases when the file is built as part of the library or
when it's built as part of a standalone executable, in which case the
dynamic API mechanism needs to be disabled.

This would pose a problem especially with MSVC which uses pre-compiled
headers (PCH) which do not sit well with the preprocessor logic that
would be needed, like the hacks added in test/testevdev.c

Using a completely separate source file for the standalone executable
avoids the issue, especially considering that the new file is not built
with MSVC at all.

The new test program can be built with:

  cmake -DCMAKE_BUILD_TYPE=Debug -DSDL_TESTS=ON -S . -B build
  make -C testdescriptor

Signed-off-by: Antonio Ospite <antonio.ospite@collabora.com>

* testdescriptor: Fix compiler warning

Fix compiler warning:

-----------------------------------------------------------------------
.../test/testdescriptor.c: In function ‘main’:
.../test/testdescriptor.c:56:17: warning: unused variable ‘file’ [-Wunused-variable]
   56 |     const char *file = argv[1];
      |                 ^~~~

-----------------------------------------------------------------------

Use the `file` variable when appropriate since it is already declared
and assigned.

Signed-off-by: Antonio Ospite <antonio.ospite@collabora.com>

* testdescriptor: Clean up descriptor context before exiting

For correctness clean up the descriptor parsing context before exiting,
even though this is not a problem in practice for a standalone
executable.

Signed-off-by: Antonio Ospite <antonio.ospite@collabora.com>

* testdescriptor: Exercise public API for parsing report descriptor

This gives some more coverage when running the SDL_report_descriptor
test program.

Signed-off-by: Antonio Ospite <antonio.ospite@collabora.com>

* SDL_report_descriptor: use size_t for variables that represent buffer sizes

Using size_t type for variables that represent buffer sizes in
src/joystick/hidapi/SDL_report_descriptor.[hc] addresses possible errors
when using the functions in combination with SDL_LoadFile, like in
test/testdescriptor.c

When building with CMake on Windows x64 the compilation was failing with
the following error:

-----------------------------------------------------------------------
D:\a\SDL\SDL\test\testdescriptor.c(69): error C2220: the following warning is treated as an error
D:\a\SDL\SDL\test\testdescriptor.c(69): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data
-----------------------------------------------------------------------

Signed-off-by: Antonio Ospite <antonio.ospite@collabora.com>

* SDL_hidapi_xboxone: protect against passing negative size to SDL_ReadReportData

Since SDL_ReadReportData() is now using size_t for the buffer size, also
change its user HIDAPI_DriverXboxOne_HandleDescriptorReport to pass
size_t, but protecting against possible cases of passing negative values
to SDL_ReadReportData().

To do that it is enough to check that the "size" argument is not zero,
so it will never be negative when decremented (size--) a few lines
below.

Note: this just addresses a theoretical issue with future callers of
HIDAPI_DriverXboxOne_HandleDescriptorReport, the current code would be
already safe as the current caller already checks that size > 0.

Signed-off-by: Antonio Ospite <antonio.ospite@collabora.com>

---------

Signed-off-by: Antonio Ospite <antonio.ospite@collabora.com>

src/joystick/hidapi/SDL_hidapi_xboxone.c

@@ -711,11 +711,15 @@ static void HandleDescriptorTrigger(Uint64 timestamp, SDL_Joystick *joystick, SD
     SDL_SendJoystickAxis(timestamp, joystick, axis, axis_value);
 }
 
-static bool HIDAPI_DriverXboxOne_HandleDescriptorReport(SDL_Joystick *joystick, SDL_DriverXboxOne_Context *ctx, Uint8 *data, int size)
+static bool HIDAPI_DriverXboxOne_HandleDescriptorReport(SDL_Joystick *joystick, SDL_DriverXboxOne_Context *ctx, Uint8 *data, size_t size)
 {
     const SDL_ReportDescriptor *descriptor = ctx->descriptor;
     Uint64 timestamp = SDL_GetTicksNS();
 
+    if (size == 0) {
+        return false;
+    }
+
     // Skip the report ID
     const Uint8 report_id = *data;
     ++data;

src/joystick/hidapi/SDL_report_descriptor.c

@@ -196,7 +196,7 @@ static void DebugMainTag(DescriptorContext *ctx, const char *tag, Uint32 flags)
 #endif // DEBUG_DESCRIPTOR
 }
 
-static Uint32 ReadValue(const Uint8 *data, int size)
+static Uint32 ReadValue(const Uint8 *data, size_t size)
 {
     Uint32 value = 0;
 
@@ -294,7 +294,7 @@ static bool AddInputFields(DescriptorContext *ctx)
     return true;
 }
 
-static bool ParseMainItem(DescriptorContext *ctx, int tag, int size, const Uint8 *data)
+static bool ParseMainItem(DescriptorContext *ctx, int tag, size_t size, const Uint8 *data)
 {
     Uint32 flags;
 
@@ -302,7 +302,9 @@ static bool ParseMainItem(DescriptorContext *ctx, int tag, int size, const Uint8
     case MainTagInput:
         flags = ReadValue(data, size);
         DebugMainTag(ctx, "MainTagInput", flags);
-        AddInputFields(ctx);
+        if (!AddInputFields(ctx)) {
+            return false;
+        }
         break;
     case MainTagOutput:
         flags = ReadValue(data, size);
@@ -357,7 +359,7 @@ static bool ParseMainItem(DescriptorContext *ctx, int tag, int size, const Uint8
     return true;
 }
 
-static bool ParseGlobalItem(DescriptorContext *ctx, int tag, int size, const Uint8 *data)
+static bool ParseGlobalItem(DescriptorContext *ctx, int tag, size_t size, const Uint8 *data)
 {
     Uint32 value;
 
@@ -414,15 +416,17 @@ static bool ParseGlobalItem(DescriptorContext *ctx, int tag, int size, const Uin
     return true;
 }
 
-static bool ParseLocalItem(DescriptorContext *ctx, int tag, int size, const Uint8 *data)
+static bool ParseLocalItem(DescriptorContext *ctx, int tag, size_t size, const Uint8 *data)
 {
     Uint32 value;
 
     switch (tag) {
     case LocalTagUsage:
         value = ReadValue(data, size);
-        AddUsage(ctx, value);
         DebugDescriptor(ctx, "LocalTagUsage: 0x%.4x", value);
+        if (!AddUsage(ctx, value)) {
+            return false;
+        }
         break;
     case LocalTagUsageMinimum:
         ctx->local.usage_minimum = ReadValue(data, size);
@@ -460,14 +464,14 @@ static bool ParseLocalItem(DescriptorContext *ctx, int tag, int size, const Uint
     return true;
 }
 
-static bool ParseDescriptor(DescriptorContext *ctx, const Uint8 *descriptor, int descriptor_size)
+static bool ParseDescriptor(DescriptorContext *ctx, const Uint8 *descriptor, size_t descriptor_size)
 {
     SDL_zerop(ctx);
 
     for (const Uint8 *here = descriptor; here < descriptor + descriptor_size; ) {
         static const int sizes[4] = { 0, 1, 2, 4 };
         Uint8 data = *here++;
-        int size = sizes[(data & 0x3)];
+        size_t size = sizes[(data & 0x3)];
         int type = ((data >> 2) & 0x3);
         int tag = (data >> 4);
 
@@ -476,7 +480,7 @@ static bool ParseDescriptor(DescriptorContext *ctx, const Uint8 *descriptor, int
         }
 
 #ifdef DEBUG_DESCRIPTOR
-        SDL_Log("Data: 0x%.2x, size: %d, type: %d, tag: %d", data, size, type, tag);
+        SDL_Log("Data: 0x%.2x, size: %zu, type: %d, tag: %d", data, size, type, tag);
 #endif
         switch (type) {
         case DescriptorItemTypeMain:
@@ -510,7 +514,7 @@ static void CleanupContext(DescriptorContext *ctx)
     SDL_free(ctx->fields);
 }
 
-SDL_ReportDescriptor *SDL_ParseReportDescriptor(const Uint8 *descriptor, int descriptor_size)
+SDL_ReportDescriptor *SDL_ParseReportDescriptor(const Uint8 *descriptor, size_t descriptor_size)
 {
     SDL_ReportDescriptor *result = NULL;
 
@@ -551,10 +555,10 @@ void SDL_DestroyDescriptor(SDL_ReportDescriptor *descriptor)
     }
 }
 
-bool SDL_ReadReportData(const Uint8 *data, int size, int bit_offset, int bit_size, Uint32 *value)
+bool SDL_ReadReportData(const Uint8 *data, size_t size, int bit_offset, int bit_size, Uint32 *value)
 {
     int offset = (bit_offset / 8);
-    if (offset >= size) {
+    if ((size_t)offset >= size) {
         *value = 0;
         return SDL_SetError("Out of bounds reading report data");
     }
@@ -585,32 +589,3 @@ bool SDL_ReadReportData(const Uint8 *data, int size, int bit_offset, int bit_siz
     }
     return true;
 }
-
-#ifdef TEST_MAIN
-
-#include <SDL3/SDL_main.h>
-
-int main(int argc, char *argv[])
-{
-    const char *file = argv[1];
-    if (argc < 2) {
-        SDL_Log("Usage: %s file", argv[0]);
-        return 1;
-    }
-
-    size_t descriptor_size = 0;
-    Uint8 *descriptor = SDL_LoadFile(argv[1], &descriptor_size);
-    if (!descriptor) {
-        SDL_Log("Couldn't load %s: %s", argv[1], SDL_GetError());
-        return 2;
-    }
-
-    DescriptorContext ctx;
-    if (!ParseDescriptor(&ctx, descriptor, descriptor_size)) {
-        SDL_Log("Couldn't parse %s: %s", argv[1], SDL_GetError());
-        return 3;
-    }
-    return 0;
-}
-
-#endif // TEST_MAIN

src/joystick/hidapi/SDL_report_descriptor.h

@@ -20,6 +20,9 @@
 */
 #include "SDL_internal.h"
 
+#ifndef SDL_report_descriptor_h_
+#define SDL_report_descriptor_h_
+
 typedef struct
 {
 	Uint8 report_id;
@@ -34,7 +37,9 @@ typedef struct
 	DescriptorInputField *fields;
 } SDL_ReportDescriptor;
 
-extern SDL_ReportDescriptor *SDL_ParseReportDescriptor(const Uint8 *descriptor, int descriptor_size);
+extern SDL_ReportDescriptor *SDL_ParseReportDescriptor(const Uint8 *descriptor, size_t descriptor_size);
 extern bool SDL_DescriptorHasUsage(SDL_ReportDescriptor *descriptor, Uint16 usage_page, Uint16 usage);
 extern void SDL_DestroyDescriptor(SDL_ReportDescriptor *descriptor);
-extern bool SDL_ReadReportData(const Uint8 *data, int size, int bit_offset, int bit_size, Uint32 *value);
+extern bool SDL_ReadReportData(const Uint8 *data, size_t size, int bit_offset, int bit_size, Uint32 *value);
+
+#endif // SDL_report_descriptor_h_

test/CMakeLists.txt

@@ -381,6 +381,9 @@ set(build_options_dependent_tests )
 
 add_sdl_test_executable(testevdev BUILD_DEPENDENT NONINTERACTIVE SOURCES testevdev.c NAME83 evdev)
 
+add_sdl_test_executable(testdescriptor BUILD_DEPENDENT SOURCES testdescriptor.c NAME83 descriptr)
+target_compile_definitions(testdescriptor PRIVATE DEBUG_DESCRIPTOR)
+
 if(MACOS)
     add_sdl_test_executable(testnative BUILD_DEPENDENT NEEDS_RESOURCES TESTUTILS
         SOURCES

test/testdescriptor.c

@@ -0,0 +1,76 @@
+/*
+  Simple DirectMedia Layer
+  Copyright (C) 1997-2026 Sam Lantinga <slouken@libsdl.org>
+
+  This software is provided 'as-is', without any express or implied
+  warranty.  In no event will the authors be held liable for any damages
+  arising from the use of this software.
+
+  Permission is granted to anyone to use this software for any purpose,
+  including commercial applications, and to alter it and redistribute it
+  freely, subject to the following restrictions:
+
+  1. The origin of this software must not be misrepresented; you must not
+     claim that you wrote the original software. If you use this software
+     in a product, an acknowledgment in the product documentation would be
+     appreciated but is not required.
+  2. Altered source versions must be plainly marked as such, and must not be
+     misrepresented as being the original software.
+  3. This notice may not be removed or altered from any source distribution.
+*/
+
+/*
+ * Standalone driver for src/joystick/hidapi/SDL_report_descriptor.c, to make
+ * it easier to experiment with HID report descriptor parsing.
+ */
+
+/* Hack #1: avoid inclusion of SDL_main.h by SDL_internal.h */
+#define SDL_main_h_
+
+/* Hack #2: avoid dynapi renaming (must be done before #include <SDL3/SDL.h>) */
+#include "../src/dynapi/SDL_dynapi.h"
+#ifdef SDL_DYNAMIC_API
+#undef SDL_DYNAMIC_API
+#endif
+#define SDL_DYNAMIC_API 0
+
+#ifdef HAVE_BUILD_CONFIG
+#include "../src/SDL_internal.h"
+#endif
+
+/* Hack #3: undo Hack #1 */
+#ifdef SDL_main_h_
+#undef SDL_main_h_
+#endif
+#ifdef SDL_MAIN_NOIMPL
+#undef SDL_MAIN_NOIMPL
+#endif
+
+#include <SDL3/SDL_main.h>
+
+#include "../src/joystick/hidapi/SDL_report_descriptor.h"
+#include "../src/joystick/hidapi/SDL_report_descriptor.c"
+
+int main(int argc, char *argv[])
+{
+    const char *file = argv[1];
+    if (argc < 2) {
+        SDL_Log("Usage: %s file", argv[0]);
+        return 1;
+    }
+
+    size_t descriptor_size = 0;
+    Uint8 *descriptor = SDL_LoadFile(file, &descriptor_size);
+    if (!descriptor) {
+        SDL_Log("Couldn't load %s: %s", file, SDL_GetError());
+        return 2;
+    }
+
+    SDL_ReportDescriptor *sdl_descriptor = SDL_ParseReportDescriptor(descriptor, descriptor_size);
+    if (sdl_descriptor == NULL) {
+        SDL_Log("Couldn't parse %s: %s", file, SDL_GetError());
+        return 3;
+    }
+    SDL_DestroyDescriptor(sdl_descriptor);
+    return 0;
+}

test/testevdev.c

@@ -171,7 +171,7 @@ static unsigned char xbox_one_elite_2_hid_report_descriptor[] =
     0x06, 0x75, 0x08, 0x15, 0x00, 0x25, 0x65, 0x05,
     0x07, 0x19, 0x00, 0x29, 0x65, 0x81, 0x00, 0xc0,
 };
-SDL_COMPILE_TIME_ASSERT (xbox_one_elite_2, sizeof (xbox_one_elite_2_hid_report_descriptor) == 0720);
+SDL_COMPILE_TIME_ASSERT (xbox_one_elite_2, sizeof (xbox_one_elite_2_hid_report_descriptor) == 464);
 
 static unsigned char ps3_hid_report_descriptor[] =
 {
@@ -263,7 +263,7 @@ static unsigned char vrs_pedals_hid_report_descriptor[] =
     0x3f, 0x75, 0x08, 0x26, 0xff, 0x00, 0x15, 0x00,
     0x09, 0x01, 0x81, 0x02, 0xc0, 0xc0,
 };
-SDL_COMPILE_TIME_ASSERT (vrs_pedals, sizeof (vrs_pedals_hid_report_descriptor) == 0136);
+SDL_COMPILE_TIME_ASSERT (vrs_pedals, sizeof (vrs_pedals_hid_report_descriptor) == 94);
 
 static unsigned char thinkpad_usb_keyboard_hid_report_descriptor[] =
 {
@@ -323,7 +323,7 @@ static unsigned char heusinkveld_pedals_hid_report_descriptor[] =
     0x75, 0x08, 0x95, 0x03, 0x09, 0x00, 0x82, 0x01,
     0x01, 0xc0,
 };
-SDL_COMPILE_TIME_ASSERT (heusinkveld_pedals, sizeof (heusinkveld_pedals_hid_report_descriptor) == 072);
+SDL_COMPILE_TIME_ASSERT (heusinkveld_pedals, sizeof (heusinkveld_pedals_hid_report_descriptor) == 58);
 
 static unsigned char fanatec_handbrake_hid_report_descriptor[] =
 {
@@ -334,7 +334,7 @@ static unsigned char fanatec_handbrake_hid_report_descriptor[] =
     0x95, 0x03, 0x81, 0x02, 0x06, 0x00, 0xff, 0x09,
     0x01, 0x95, 0x02, 0x91, 0x02, 0xc0,
 };
-SDL_COMPILE_TIME_ASSERT (fanatec_handbrake, sizeof (fanatec_handbrake_hid_report_descriptor) == 046);
+SDL_COMPILE_TIME_ASSERT (fanatec_handbrake, sizeof (fanatec_handbrake_hid_report_descriptor) == 38);
 
 static unsigned char xpadneo09_xb1s_hid_report_descriptor[] =
 {