Repository: libsdl-org/SDL
File: src/core/linux/SDL_dbus.c (C)

Code before:
                    if (save_reply) {
                        *save_reply = reply;
                    } else {
                        dbus.message_unref(reply);
                    }
                }
            }
            va_end(ap_reply);
            dbus.message_unref(msg);
        }
    }

    return result;
}

bool SDL_DBus_CallMethodOnConnection(DBusConnection *conn, const char *node, const char *path, const char *interface, const char *method, ...)
{
    bool result;
    va_list ap;
    va_start(ap, method);
    result = SDL_DBus_CallMethodInternal(conn, node, path, interface, method, NULL, ap);
    va_end(ap);
    return result;
}

bool SDL_DBus_CallMethodOnConnectionWithReply(DBusConnection *conn, const char *node, const char *path, const char *interface, const char *method, DBusMessage **save_reply, ...)
{
    bool result;
    va_list ap;
    va_start(ap, save_reply);
    result = SDL_DBus_CallMethodInternal(conn, node, path, interface, method, save_reply, ap);
    va_end(ap);
    return result;
}

bool SDL_DBus_CallMethod(const char *node, const char *path, const char *interface, const char *method, ...)
{
    bool result;
    va_list ap;
    va_start(ap, method);
    result = SDL_DBus_CallMethodInternal(dbus.session_conn, node, path, interface, method, NULL, ap);
    va_end(ap);
    return result;
}

static bool SDL_DBus_CallVoidMethodInternal(DBusConnection *conn, const char *node, const char *path, const char *interface, const char *method, va_list ap)
{
    bool result = false;

    if (conn) {
        DBusMessage *msg = dbus.message_new_method_call(node, path, interface, method);

Review comment (suggestion):
If it was my code, I'd add the `save_reply` parameter to `SDL_DBus_CallMethodOnConnection()` rather than creating a new `SDL_DBus_CallMethodOnConnectionWithReply()`, because that gives you a perfect opportunity to go through all the calls to `SDL_DBus_CallMethodOnConnection()` and assess whether they need to hold onto the reply. There aren't *that* many of them.

With this pattern, it seems error-prone - it would be very easy to call `SDL_DBus_CallMethodOnConnection()` with arguments that do try to retrieve strings from the message and then use-after-free those strings.

If the `DBusMessage **` was further away from the varargs - perhaps immediately after the `DBusConnection *` - then the compiler would be more likely to tell you anywhere that you had missed adding it (because you'd be trying to pass a `const char *` where a `DBusMessage **` was expected).

(And if there's any doubt about whether the call needs to hold onto the message, I would personally err on the side of doing so until after you've finished doing things with its result, otherwise it would be very easy to copypaste code to call a method that doesn't return a string or object path and use it to call a method that *does* return a string or object path, and you're back to having a use-after-free.)

Code after:
                        result = true;
                    }
                    if (save_reply) {
                        *save_reply = reply;
                    } else {
                        dbus.message_unref(reply);
                    }
                }
            }
            va_end(ap_reply);
            dbus.message_unref(msg);
        }
    }

    return result;
}

bool SDL_DBus_CallMethodOnConnection(DBusConnection *conn, DBusMessage **save_reply, const char *node, const char *path, const char *interface, const char *method, ...)
{
    bool result;
    va_list ap;
    va_start(ap, method);
    result = SDL_DBus_CallMethodInternal(conn, save_reply, node, path, interface, method, ap);
    va_end(ap);
    return result;
}

bool SDL_DBus_CallMethod(DBusMessage **save_reply, const char *node, const char *path, const char *interface, const char *method, ...)
{
    bool result;
    va_list ap;
    va_start(ap, method);
    result = SDL_DBus_CallMethodInternal(dbus.session_conn, save_reply, node, path, interface, method, ap);
    va_end(ap);
    return result;
}

static bool SDL_DBus_CallVoidMethodInternal(DBusConnection *conn, const char *node, const char *path, const char *interface, const char *method, va_list ap)
{
    bool result = false;

    if (conn) {
        DBusMessage *msg = dbus.message_new_method_call(node, path, interface, method);
        if (msg) {
            int firstarg = va_arg(ap, int);
            if ((firstarg == DBUS_TYPE_INVALID) || dbus.message_append_args_valist(msg, firstarg, ap)) {
                dbus.message_set_no_reply(msg, true);
                if (dbus.connection_send(conn, msg, NULL)) {
                    dbus.connection_flush(conn);
                    result = true;
                }