[PATCH 0/1] MR10931: winebth: Fix a memory leak in bluez_gatt_characteristic_read.

[Alex Henrie (＠alexhenrie)]

The leak was identified by Cursor, which is a mix of LLM models. I wrote the fix myself.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10931

[PATCH 1/1] winebth: Fix a memory leak in bluez_gatt_characteristic_read.

[Alex Henrie]

From: Alex Henrie <[e-mail]>

---
 dlls/winebth.sys/dbus.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/dlls/winebth.sys/dbus.c b/dlls/winebth.sys/dbus.c
index 4431f527f73..e995b760535 100644
--- a/dlls/winebth.sys/dbus.c
+++ b/dlls/winebth.sys/dbus.c
@@ -1111,7 +1111,7 @@ NTSTATUS bluez_gatt_characteristic_read( void *connection, void *watcher_ctx, st
                                          IRP *irp )
 {
     DBusMessageIter args_iter, dict_iter = DBUS_MESSAGE_ITER_INIT_CLOSED;
-    struct bluez_async_req_data *data = NULL;
+    struct bluez_async_req_data *data;
     DBusPendingCall *pending_call = NULL;
     DBusMessage *request;
     NTSTATUS status;
@@ -1123,14 +1123,7 @@ NTSTATUS bluez_gatt_characteristic_read( void *connection, void *watcher_ctx, st
                                               "ReadValue" );
     if (!request)
         return STATUS_NO_MEMORY;
-    if (!(data = malloc( sizeof( *data ) )))
-    {
-        status = STATUS_NO_MEMORY;
-        goto failed;
-    }
 
-    data->irp = irp;
-    data->watcher_ctx = watcher_ctx;
     p_dbus_message_iter_init_append( request, &args_iter );
     if (!p_dbus_message_iter_open_container( &args_iter, DBUS_TYPE_ARRAY, "{sv}", &dict_iter ))
     {
@@ -1153,8 +1146,16 @@ NTSTATUS bluez_gatt_characteristic_read( void *connection, void *watcher_ctx, st
         status = STATUS_INTERNAL_ERROR;
         goto failed;
     }
+    if (!(data = malloc( sizeof( *data ) )))
+    {
+        status = STATUS_NO_MEMORY;
+        goto failed;
+    }
+    data->irp = irp;
+    data->watcher_ctx = watcher_ctx;
     if (!p_dbus_pending_call_set_notify( pending_call, bluez_gatt_characteristic_read_callback, data, free ))
     {
+        free( data );
         p_dbus_pending_call_cancel( pending_call );
         p_dbus_pending_call_unref( pending_call );
         status = STATUS_NO_MEMORY;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/10931

[Vibhav Pant (＠vibhavp)]

Vibhav Pant (@vibhavp) commented about dlls/winebth.sys/dbus.c:
> IRP *irp )
>  {
>      DBusMessageIter args_iter, dict_iter = DBUS_MESSAGE_ITER_INIT_CLOSED;
> -    struct bluez_async_req_data *data = NULL;

I think it would be much simpler to let the old code stay, and just add `free(data)` under `failed`. `free(NULL)` is guaranteed to do nothing.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10931#note_141721

[Alex Henrie (＠alexhenrie)]

On Fri May 29 16:38:54 2026 +0000, Vibhav Pant wrote:
> I think it would be much simpler to let the old code stay, and just add
> `free(data)` under `failed`. `free(NULL)` is guaranteed to do nothing.
> I thought about doing that, then realized that if we wait to initialize `data` until it is actually needed then `free(data)` only needs to be called in the last error case and not in every error case. In my opinion that makes the code more clear. But either way, initializing `data` to `NULL` is redundant because `data = malloc(sizeof(*data))` is always run before `free(data)` is called.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10931#note_141724

[Vibhav Pant (＠vibhavp)]

On Fri May 29 16:50:29 2026 +0000, Alex Henrie wrote:
> I thought about doing that, then realized that if we wait to allocate
> `data` until it is actually needed then `free(data)` only needs to be
> called in the last error case and not in every error case. In my opinion
> that makes the code more clear. But either way, initializing `data` to
> `NULL` is redundant because `data = malloc(sizeof(*data))` is always run
> before `free(data)` is called.
> Ah, I see. That's fair as well.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10931#note_141727

[PATCH 0/1] MR10931: winebth: Fix a memory leak in bluez_gatt_characteristic_read. - approved

[Vibhav Pant (＠vibhavp)]

This merge request was approved by Vibhav Pant.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10931