Should destroy functions take pointer to pointer?

[slouken]

What do you guys think about having the destroy functions take a pointer to an object pointer, so it can automatically NULL it out?
e.g.

void SDL_DestroySurface(SDL_Surface **surfacep)
{
    if (!surfacep || !*surfacep) {
        return;
    }

    ...
    *surfacep = NULL;
}

@libsdl-org/a-team, @smcv, thoughts?

[sezero]

Vaguely sounds like microsoft's oh-so-very-safe crt apis? I'm not strongly objecting but do we really need this?

[icculus]

Hard no from me.

[slouken]

Fair enough. It would probably also make language bindings harder.

[smcv]

The convention used in e.g. GLib and libdbus is that destroy/free/unref functions take the pointer like they do in SDL2, but for authors of C code it's extremely convenient to have a macro that implements this pattern (shown here as pseudocode using GLib's naming convention):

void mylib_thing_free (MylibThing *thing);

void mylib_clear_thing(MylibThing **thingp)
{
    MylibThing *thing = *thingp;

    *thingp = NULL;
    if (thing) {
        mylib_thing_free(thing);
    }
}

int do_stuff (...)
{
    MylibThing *thing = NULL;
    int ret = 0;

    ... do something that might allocate it ...

    ... do something that might set ret = -1 and 'goto out' ...

out:
    mylib_clear_thing(&thing);
    return ret;
}

For example GLib has this for GObject subclasses as the pair g_object_unref()/g_clear_object(), and generically for whatever type of pointer as g_clear_pointer(&thing, thing_free). Recent libdbus has dbus_clear_connection(&conn), dbus_clear_message(&msg) and so on following the same pattern.

CPython Py_CLEAR() is similar, although it hides the extra & inside a macro.

This is particularly convenient for users of compilers implementing gcc-compatible __attribute__((cleanup)). SDL can't rely on this itself, the same way GLib can't, because SDL and GLib are portable to compilers like MSVC that don't implement that extension; but there are plenty of GLib-dependent projects like Flatpak that only care about gcc-compatible compilers, and similarly I'm sure there are plenty of SDL-dependent projects that only care about gcc-compatible compilers.

[slouken]

Hmmmmm, we do know the type of the pointer in SDL3 now, we could potentially implement SDL_ClearObject(void **foo)

[slouken]

Actually, no, that wouldn't work well. You'd have to cast your argument to (void **), never mind.

[smcv]

It would have to be SDL_ClearObject(void *pointer_to_pointer) and give up compile-time type-safety, because passing a SDL_Surface ** where a void ** is expected violates ISO C "strict aliasing" and compilers will complain accordingly.

It might be better to have the equivalent of what libdbus does: SDL_ClearSurface(SDL_Surface **) and so on. libdbus has this macro:

/* Implementation for dbus_clear_message() etc. This is not API,
 * do not use it directly.
 *
 * We're using a specific type (T ** and T *) instead of void ** and
 * void * partly for type-safety, partly to be strict-aliasing-compliant,
 * and partly to keep C++ compilers happy. This code is inlined into
 * users of libdbus, so we can't rely on it having dbus' own compiler
 * settings. */
#define _dbus_clear_pointer_impl(T, pointer_to_pointer, destroy) \
  do { \
    T **_pp = (pointer_to_pointer); \
    T *_value = *_pp; \
    \
    *_pp = NULL; \
    \
    if (_value != NULL) \
      destroy (_value); \
  } while (0)
/* Not (destroy) (_value) in case destroy() is a function-like macro */

which is used like this:

static inline void
dbus_clear_connection (DBusConnection **pointer_to_connection)
{
  _dbus_clear_pointer_impl (DBusConnection, pointer_to_connection,
                            dbus_connection_unref);
}

The implementation was https://gitlab.freedesktop.org/dbus/dbus/-/commit/e13f29cae788ee0d0d16352c9be2d9e14a5a0b3d, © 2017 Collabora Ltd. and I'm happy to license it under the Zlib license if you want.

[madebr]

Can we use _Generic macros here?
Only problem is it is C11+.