a11y-keyboard-atspi: Fix memory leak

[cwendling]

Fix fairly large memory leak when beeping on keys while caps lock is enabled. The libatspi2 docs and API were quite misleading, so I overlooked the fact the event parameter should be freed in the callback.

This changes the constness of the callback argument, which is new in libatspi2 2.40 -- yet the actual behavior didn't change, only the qualifier was removed, see [1]. This might however bring up a compiler warning when building against libatspi2 < 2.40; but on the other hand it fixed build with clang >= 16, see #399. As it is unlikely to build with clang >= 16 and libatspi2 < 2.40, I think it's a good compromise.

[1] https://gitlab.gnome.org/GNOME/at-spi2-core/-/commit/7dfb0b7fc2d1710ef7fad54f910fa4c6a5e3af17

[cwendling]

If we don't want the "issue" with libatspi2 < 2.40, we could do something like this:

diff --git a/configure.ac b/configure.ac
index 1a82c90..b21c1e4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -133,7 +133,10 @@ AS_IF([test "x$with_libatspi" != xno],
              [AC_MSG_ERROR([libatspi support requested but libraries not found])])
        PKG_CHECK_EXISTS([atspi-2 > 2.36.0],
                         [AC_DEFINE([DESTROYING_ATSPI_LISTENER_DOES_NOT_CRASH], [1], [Define if libatspi does not have bug 22])],
-                        [])])
+                        [])
+       PKG_CHECK_EXISTS([atspi-2 > 2.39.0],
+                        [],
+                        [AC_DEFINE([CONST_ATSPI_DEVICE_LISTENER], [1], [Define if libatspi DeviceEventCB takes a constant argument])])])
 AM_CONDITIONAL([HAVE_LIBATSPI], [test "x$have_libatspi" = xyes])

 dnl ---------------------------------------------------------------------------
diff --git a/plugins/a11y-keyboard/msd-a11y-keyboard-atspi.c b/plugins/a11y-keyboard/msd-a11y-keyboard-atspi.c
index a45ef47..b3a2bdb 100644
--- a/plugins/a11y-keyboard/msd-a11y-keyboard-atspi.c
+++ b/plugins/a11y-keyboard/msd-a11y-keyboard-atspi.c
@@ -54,9 +54,15 @@ msd_a11y_keyboard_atspi_class_init (MsdA11yKeyboardAtspiClass *klass)
         object_class->finalize = msd_a11y_keyboard_atspi_finalize;
 }

+#ifdef CONST_ATSPI_DEVICE_LISTENER
+#       define EVENT_QUALIFIER const
+#else
+#       define EVENT_QUALIFIER /* nothing */
+#endif
+
 static gboolean
-on_key_press_event (AtspiDeviceEvent *event,
-                    void             *user_data G_GNUC_UNUSED)
+on_key_press_event (EVENT_QUALIFIER AtspiDeviceEvent *event,
+                    void                             *user_data G_GNUC_UNUSED)
 {
         /* don't ring on capslock itself, that's taken care of by togglekeys
          * if the user want it. */

However, I feel like it might be a little too much for just that warning when building with older libatspi2…

[raveit65]

Actual fedora release use at-spi2-atk.x86_64-2.48.3-1.fc38. I there any distro which use < 2.40 ? I guess only debian....... :)

[cwendling]

I there any distro which use < 2.40 ? I guess only debian....... :)

Not even, Debian released v12 recently so now it has 2.46 :) But yeah, oldstable has 2.38.

runtime test […] was hopefully done by author of PR.

It was indeed, and I confirmed it fixed a leak I never noticed (but which was fairly bad actually).