chore(linux): fix memory allocation in keymanutil.c

Also add unit tests for keyman_put_options_todconf.

linux/ibus-keyman/.gitignore

@@ -5,5 +5,7 @@ src/keyman-version.h
 src/ibus-engine-keyman
 src/print_kmp
 src/print_kmpdetails
+src/test/keymanutil_tests
 *.log
 *.trs
+.dirstamp

linux/ibus-keyman/configure.ac

@@ -100,6 +100,7 @@ Makefile
 src/Makefile
 src/keyman.xml.in
 src/keyman-version.h
+src/test/Makefile
 tests/Makefile
 icons/Makefile
 m4/Makefile

linux/ibus-keyman/src/Makefile.am

@@ -18,6 +18,10 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 
+SUBDIRS = \
+  test \
+  $(NULL)
+
 AM_CFLAGS = \
 	@IBUS_CFLAGS@ \
 	@GTK_CFLAGS@ \

linux/ibus-keyman/src/keymanutil.c

@@ -376,7 +376,8 @@ keyman_get_options_queue_fromdconf(gchar *package_id,
 
 // Write new keyboard option to DConf.
 // DConf options are in a list of strings like ['option_key1=value1', 'option_key2=value2']
-// If the option key already exists, the value is updated. Otherwise a new string 'option_key=option_value' is appended.
+// If the option key already exists, the value is updated. Otherwise a new string
+// 'option_key=option_value' is appended.
 //
 // Parameters:
 // package_id   (gchar *): Package ID
@@ -420,17 +421,20 @@ keyman_put_options_todconf(gchar *package_id,
         if (!option_updated)
         {
             // Resize to add new option and null-terminate
-            options = g_realloc(options, strlen(kvp) + 1);
+            int size = index + 2; // old size: index + 1, plus 1 new
+            options = g_renew(gchar*, options, size);
             options[index] = kvp;
             options[index+1] = NULL;
         }
     }
     else
     {
-        // If options don't exist, allocate space for new option and null-terminate
-        options = g_malloc(strlen(kvp) + 1);
-        options[0] = kvp;
-        options[1] = NULL;
+      // we never should come here - keyman_get_options_fromdconf will create empty
+      // options if they don't yet exist.
+      // Allocate space for new option and null-terminate
+      options    = g_new(gchar *, 2);
+      options[0] = kvp;
+      options[1] = NULL;
     }
 
     // Write to DConf

linux/ibus-keyman/src/test/Makefile.am

@@ -0,0 +1,78 @@
+# vim:set noet ts=4:
+#
+# ibus-keyman - The Keyman engine for IBus
+#
+# Copyright (c) 2021 SIL International
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+AUTOMAKE_OPTIONS = subdir-objects
+
+AM_CFLAGS = \
+  @IBUS_CFLAGS@ \
+  @GTK_CFLAGS@ \
+  @JSON_GLIB_CFLAGS@ \
+  @X11_CFLAGS@ \
+  @KEYMAN_PROC_CFLAGS@ \
+  -DPKGDATADIR=\"$(pkgdatadir)\" \
+  $(NULL)
+
+AM_LDFLAGS = \
+  @IBUS_LIBS@ \
+  @GTK_LIBS@ \
+  @X11_LIBS@ \
+  @JSON_GLIB_LIBS@ \
+  @KEYMAN_PROC_LIBS@ \
+  $(NULL)
+
+check_PROGRAMS = \
+  keymanutil_tests \
+  $(NULL)
+
+keymanutil_tests_SOURCES = \
+  keymanutil_tests.c \
+  ../keymanutil.c \
+  ../keymanutil.h \
+  ../kmpdetails.c \
+  ../kmpdetails.h \
+  $(NULL)
+
+keymanutil_tests_CFLAGS = \
+  @IBUS_CFLAGS@ \
+  @GTK_CFLAGS@ \
+  @X11_CFLAGS@ \
+  @JSON_GLIB_CFLAGS@ \
+  @KEYMAN_PROC_CFLAGS@ \
+  -DPKGDATADIR=\"$(pkgdatadir)\" \
+  -I$(top_srcdir)/src \
+  $(NULL)
+
+keymanutil_tests_CPPFLAGS = \
+  @IBUS_CFLAGS@ \
+  @GTK_CFLAGS@ \
+  @X11_CFLAGS@ \
+  @JSON_GLIB_CFLAGS@ \
+  @KEYMAN_PROC_CFLAGS@ \
+  -DPKGDATADIR=\"$(pkgdatadir)\" \
+  -I$(top_srcdir)/src \
+  $(NULL)
+
+keymanutil_tests_LDFLAGS = \
+  @GTK_LIBS@ \
+  @IBUS_LIBS@ \
+  @X11_LIBS@ \
+  @JSON_GLIB_LIBS@ \
+  @KEYMAN_PROC_LIBS@ \
+  $(NULL)

linux/ibus-keyman/src/test/keymanutil_tests.c

@@ -0,0 +1,130 @@
+#include <glib-object.h>
+#include <glib.h>
+#include <gtk/gtk.h>
+#include "keymanutil.h"
+
+#define TEST_FIXTURE "keymanutil-test"
+
+void
+delete_key(gchar* testname) {
+  gchar *path = g_strdup_printf("%s%s/%s/", KEYMAN_DCONF_PATH, TEST_FIXTURE, testname);
+  GSettings *settings = g_settings_new_with_path(KEYMAN_CHILD_DCONF_NAME, path);
+  g_settings_reset(settings, KEYMAN_DCONF_OPTIONS_KEY);
+  g_object_unref(G_OBJECT(settings));
+  g_free(path);
+}
+
+void
+set_key(gchar* testname, gchar** options) {
+  gchar *path = g_strdup_printf("%s%s/%s/", KEYMAN_DCONF_PATH, TEST_FIXTURE, testname);
+  GSettings *settings = g_settings_new_with_path(KEYMAN_CHILD_DCONF_NAME, path);
+  g_settings_set_strv(settings, KEYMAN_DCONF_OPTIONS_KEY, (const gchar* const*)options);
+  g_object_unref(G_OBJECT(settings));
+  g_free(path);
+}
+
+gchar**
+get_key(gchar* testname) {
+  gchar* path = g_strdup_printf("%s%s/%s/", KEYMAN_DCONF_PATH, TEST_FIXTURE, testname);
+  GSettings* settings = g_settings_new_with_path(KEYMAN_CHILD_DCONF_NAME, path);
+  gchar** result = g_settings_get_strv(settings, KEYMAN_DCONF_OPTIONS_KEY);
+  g_object_unref(G_OBJECT(settings));
+  g_free(path);
+  return result;
+}
+
+void
+test_keyman_put_options_todconf__new_key() {
+  // Initialize
+  gchar* testname = "test_keyman_put_options_todconf__new_key";
+  delete_key(testname);
+  gchar* value = g_strdup_printf("%d", g_test_rand_int());
+
+  // Execute
+  keyman_put_options_todconf(TEST_FIXTURE, testname, "new_key", value);
+
+  // Verify
+  gchar** options = get_key(testname);
+  gchar* expected = g_strdup_printf("new_key=%s", value);
+  g_assert_nonnull(options);
+  g_assert_cmpstr(options[0], ==, expected);
+  g_assert_null(options[1]);
+
+  // Cleanup
+  g_free(expected);
+  g_free(value);
+  g_strfreev(options);
+  delete_key(testname);
+}
+
+void
+test_keyman_put_options_todconf__other_keys() {
+  // Initialize
+  gchar* testname = "test_keyman_put_options_todconf__other_keys";
+  delete_key(testname);
+  gchar* existingKeys[] = {"key1=val1", "key2=val2"};
+  set_key(testname, existingKeys);
+  gchar* value = g_strdup_printf("%d", g_test_rand_int());
+
+  // Execute
+  keyman_put_options_todconf(TEST_FIXTURE, testname, "new_key", value);
+
+  // Verify
+  gchar** options = get_key(testname);
+  gchar* expected = g_strdup_printf("new_key=%s", value);
+  g_assert_nonnull(options);
+  g_assert_cmpstr(options[0], ==, "key1=val1");
+  g_assert_cmpstr(options[1], ==, "key2=val2");
+  g_assert_cmpstr(options[2], ==, expected);
+  g_assert_null(options[3]);
+
+  // Cleanup
+  g_free(expected);
+  g_free(value);
+  g_strfreev(options);
+  delete_key(testname);
+}
+
+void
+test_keyman_put_options_todconf__existing_key() {
+  // Initialize
+  gchar* testname = "test_keyman_put_options_todconf__existing_key";
+  delete_key(testname);
+  gchar* existingKeys[] = {"key1=val1", "new_key=val2"};
+  set_key(testname, existingKeys);
+  gchar* value = g_strdup_printf("%d", g_test_rand_int());
+
+  // Execute
+  keyman_put_options_todconf(TEST_FIXTURE, testname, "new_key", value);
+
+  // Verify
+  gchar** options = get_key(testname);
+  gchar* expected = g_strdup_printf("new_key=%s", value);
+  g_assert_nonnull(options);
+  g_assert_cmpstr(options[0], ==, "key1=val1");
+  g_assert_cmpstr(options[1], ==, expected);
+  g_assert_null(options[2]);
+
+  // Cleanup
+  g_free(expected);
+  g_free(value);
+  g_strfreev(options);
+  delete_key(testname);
+}
+
+int
+main(int argc, char* argv[]) {
+  gtk_init(&argc, &argv);
+  g_test_init(&argc, &argv, NULL);
+  g_test_set_nonfatal_assertions();
+
+  // Add tests
+  g_test_add_func("/keymanutil/keyman_put_options_todconf/new_key", test_keyman_put_options_todconf__new_key);
+  g_test_add_func("/keymanutil/keyman_put_options_todconf/other_keys", test_keyman_put_options_todconf__other_keys);
+  g_test_add_func("/keymanutil/keyman_put_options_todconf/existing_key", test_keyman_put_options_todconf__existing_key);
+
+  // Run tests
+  int retVal = g_test_run();
+
+  return retVal;
+}