sdgui: load dictionaries in parallel, as sdtui did

Also, resolve some use-after-frees in GTK+.

src/sdgui.c

@@ -26,15 +26,6 @@
 #include "utils.h"
 #include "stardict-view.h"
 
-typedef struct dictionary Dictionary;
-
-struct dictionary
-{
-	const gchar  *filename;          ///< Filename
-	StardictDict *dict;              ///< Stardict dictionary data
-	gchar        *name;              ///< Name to show
-};
-
 static struct
 {
 	GtkWidget    *window;            ///< Top-level window
@@ -43,8 +34,7 @@ static struct
 	GtkWidget    *view;              ///< Entries view
 
 	gint          dictionary;        ///< Index of the current dictionary
-	Dictionary   *dictionaries;      ///< All open dictionaries
-	gsize         dictionaries_len;  ///< Total number of dictionaries
+	GPtrArray    *dictionaries;      ///< All open dictionaries
 
 	gboolean      watch_selection;   ///< Following X11 PRIMARY?
 }
@@ -52,31 +42,15 @@ g;
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 
-static gboolean
-dictionary_load (Dictionary *self, GError **e)
-{
-	if (!(self->dict = stardict_dict_new (self->filename, e)))
-		return FALSE;
-
-	if (!self->name)
-	{
-		self->name = g_strdup (stardict_info_get_book_name
-			(stardict_dict_get_info (self->dict)));
-	}
-	return TRUE;
-}
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-
 static void
 init (gchar **filenames)
 {
-	while (filenames[g.dictionaries_len])
-		g.dictionaries_len++;
-
-	g.dictionaries = g_malloc0_n (sizeof *g.dictionaries, g.dictionaries_len);
-	for (gsize i = 0; i < g.dictionaries_len; i++)
-		g.dictionaries[i].filename = filenames[i];
+	for (gsize i = 0; filenames[i]; i++)
+	{
+		Dictionary *dict = g_malloc0 (sizeof *dict);
+		dict->filename = g_strdup (filenames[i]);
+		g_ptr_array_add (g.dictionaries, dict);
+	}
 }
 
 // TODO: try to deduplicate, similar to app_load_config_values()
@@ -84,19 +58,21 @@ static gboolean
 init_from_key_file (GKeyFile *kf, GError **error)
 {
 	const gchar *dictionaries = "Dictionaries";
-	gchar **names =
-		g_key_file_get_keys (kf, dictionaries, &g.dictionaries_len, NULL);
+	gchar **names = g_key_file_get_keys (kf, dictionaries, NULL, NULL);
 	if (!names)
 		return TRUE;
 
-	g.dictionaries = g_malloc0_n (sizeof *g.dictionaries, g.dictionaries_len);
-	for (gsize i = 0; i < g.dictionaries_len; i++)
-		g.dictionaries[i].name = names[i];
+	for (gsize i = 0; names[i]; i++)
+	{
+		Dictionary *dict = g_malloc0 (sizeof *dict);
+		dict->name = names[i];
+		g_ptr_array_add (g.dictionaries, dict);
+	}
 	g_free (names);
 
-	for (gsize i = 0; i < g.dictionaries_len; i++)
+	for (gsize i = 0; i < g.dictionaries->len; i++)
 	{
-		Dictionary *dict = &g.dictionaries[i];
+		Dictionary *dict = g_ptr_array_index (g.dictionaries, i);
 		gchar *path =
 			g_key_file_get_string (kf, dictionaries, dict->name, error);
 		if (!path)
@@ -142,7 +118,7 @@ search (Dictionary *dict)
 static void
 on_changed (G_GNUC_UNUSED GtkWidget *widget, G_GNUC_UNUSED gpointer data)
 {
-	search (&g.dictionaries[g.dictionary]);
+	search (g_ptr_array_index (g.dictionaries, g.dictionary));
 }
 
 static void
@@ -178,7 +154,7 @@ on_switch_page (G_GNUC_UNUSED GtkWidget *widget, G_GNUC_UNUSED GtkWidget *page,
 	guint page_num, G_GNUC_UNUSED gpointer data)
 {
 	g.dictionary = page_num;
-	search (&g.dictionaries[g.dictionary]);
+	search (g_ptr_array_index (g.dictionaries, g.dictionary));
 }
 
 static gboolean
@@ -256,17 +232,19 @@ main (int argc, char *argv[])
 		return 1;
 	}
 
+	g.dictionaries =
+		g_ptr_array_new_with_free_func ((GDestroyNotify) dictionary_destroy);
 	if (filenames)
 		init (filenames);
 	else if (!init_from_config (&error) && error)
 		die_with_dialog (error->message);
+	g_strfreev (filenames);
 
-	for (gsize i = 0; i < g.dictionaries_len; i++)
-		if (!dictionary_load (&g.dictionaries[i], &error))
-			die_with_dialog (error->message);
-	if (!g.dictionaries_len)
+	if (!g.dictionaries->len)
 		die_with_dialog (_("No dictionaries found either in "
 			"the configuration or on the command line"));
+	if (!load_dictionaries (g.dictionaries, &error))
+		die_with_dialog (error->message);
 
 	// Some Adwaita stupidity
 	const char *style = "notebook header tab { padding: 2px 8px; margin: 0; }";
@@ -331,9 +309,9 @@ main (int argc, char *argv[])
 	g.view = stardict_view_new ();
 	gtk_box_pack_end (GTK_BOX (superbox), g.view, TRUE, TRUE, 0);
 
-	for (gsize i = 0; i < g.dictionaries_len; i++)
+	for (gsize i = 0; i < g.dictionaries->len; i++)
 	{
-		Dictionary *dict = &g.dictionaries[i];
+		Dictionary *dict = g_ptr_array_index (g.dictionaries, i);
 		GtkWidget *dummy = gtk_box_new (GTK_ORIENTATION_VERTICAL, 0);
 		GtkWidget *label = gtk_label_new (dict->name);
 		gtk_notebook_append_page (GTK_NOTEBOOK (g.notebook), dummy, label);
@@ -346,7 +324,5 @@ main (int argc, char *argv[])
 	gtk_widget_grab_focus (g.entry);
 	gtk_widget_show_all (g.window);
 	gtk_main ();
-
-	g_strfreev (filenames);
 	return 0;
 }

src/sdtui.c

@@ -125,7 +125,7 @@ struct attrs
 /// Data relating to one entry within the dictionary.
 typedef struct view_entry               ViewEntry;
 /// Data relating to a dictionary file.
-typedef struct dictionary               Dictionary;
+typedef struct app_dictionary           AppDictionary;
 /// Encloses application data.
 typedef struct application              Application;
 
@@ -136,12 +136,10 @@ struct view_entry
 	GPtrArray * formatting;             ///< chtype * or NULL per definition
 };
 
-struct dictionary
+struct app_dictionary
 {
-	gchar         * name;               ///< Visible identifier
-	gsize           name_width;         ///< Visible width of the name
-	gchar         * filename;           ///< Path to the dictionary
-	StardictDict  * dict;               ///< Dictionary
+	Dictionary  super;                  ///< Superclass
+	gsize       name_width;             ///< Visible width of the name
 };
 
 struct application
@@ -153,7 +151,7 @@ struct application
 	gboolean        locale_is_utf8;     ///< The locale is Unicode
 	gboolean        focused;            ///< Whether the terminal has focus
 
-	GArray        * dictionaries;       ///< All loaded dictionaries
+	GPtrArray     * dictionaries;       ///< All loaded AppDictionaries
 
 	StardictDict  * dict;               ///< The current dictionary
 	guint           show_help : 1;      ///< Whether help can be shown
@@ -352,42 +350,6 @@ view_entry_free (ViewEntry *ve)
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 
-static gboolean
-dictionary_load (Dictionary *self, Application *app, GError **e)
-{
-	if (!(self->dict = stardict_dict_new (self->filename, e)))
-		return FALSE;
-
-	if (!self->name)
-	{
-		self->name = g_strdup (stardict_info_get_book_name
-			(stardict_dict_get_info (self->dict)));
-	}
-
-	// Add some padding for decorative purposes
-	gchar *tmp = g_strdup_printf (" %s ", self->name);
-	g_free (self->name);
-	self->name = tmp;
-
-	gunichar *ucs4 = g_utf8_to_ucs4_fast (self->name, -1, NULL);
-	for (gunichar *it = ucs4; *it; it++)
-		self->name_width += app_char_width (app, *it);
-	g_free (ucs4);
-	return TRUE;
-}
-
-static void
-dictionary_free (Dictionary *self)
-{
-	g_free (self->name);
-	g_free (self->filename);
-
-	if (self->dict)
-		g_object_unref (self->dict);
-}
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-
 /// Reload view items.
 static void
 app_reload_view (Application *self)
@@ -491,8 +453,10 @@ app_load_config_values (Application *self, GKeyFile *kf)
 		else
 			resolved = path;
 
-		Dictionary dict = { .name = g_strdup (*it), .filename = resolved };
-		g_array_append_val (self->dictionaries, dict);
+		AppDictionary *dict = g_malloc0 (sizeof *dict);
+		dict->super.name = g_strdup (*it);
+		dict->super.filename = resolved;
+		g_ptr_array_add (self->dictionaries, dict);
 	}
 	g_strfreev (names);
 }
@@ -521,67 +485,29 @@ app_init_attrs (Application *self)
 #undef XX
 }
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-
 static gboolean
 app_load_dictionaries (Application *self, GError **e)
 {
-	for (guint i = 0; i < self->dictionaries->len; i++)
-		if (!dictionary_load (&g_array_index (self->dictionaries,
-			Dictionary, i), self, e))
-			return FALSE;
-	return TRUE;
-}
-
-// Parallelize dictionary loading if possible, because of collation reindexing
-#if GLIB_CHECK_VERSION (2, 36, 0)
-struct load_ctx
-{
-	Application *self;                  ///< Application context
-	GAsyncQueue *error_queue;           ///< Errors
-};
-
-static void
-app_load_worker (gpointer data, gpointer user_data)
-{
-	struct load_ctx *ctx = user_data;
-	GError *e = NULL;
-	dictionary_load (&g_array_index (ctx->self->dictionaries, Dictionary,
-		GPOINTER_TO_UINT (data) - 1), ctx->self, &e);
-	if (e)
-		g_async_queue_push (ctx->error_queue, e);
-}
+	if (!load_dictionaries (self->dictionaries, e))
+		return FALSE;
 
-static gboolean
-app_load_dictionaries_parallel (Application *self, GError **e)
-{
-	struct load_ctx ctx;
-	GThreadPool *pool = g_thread_pool_new (app_load_worker, &ctx,
-		g_get_num_processors (), TRUE, NULL);
-	if G_UNLIKELY (!g_thread_pool_get_num_threads (pool))
+	for (gsize i = 0; i < self->dictionaries->len; i++)
 	{
-		g_thread_pool_free (pool, TRUE, TRUE);
-		return app_load_dictionaries (self, e);
-	}
-
-	ctx.self = self;
-	ctx.error_queue = g_async_queue_new_full ((GDestroyNotify) g_error_free);
-	for (guint i = 0; i < self->dictionaries->len; i++)
-		g_thread_pool_push (pool, GUINT_TO_POINTER (i + 1), NULL);
+		AppDictionary *dict = g_ptr_array_index (self->dictionaries, i);
 
-	g_thread_pool_free (pool, FALSE, TRUE);
+		// Add some padding for decorative purposes
+		gchar *tmp = g_strdup_printf (" %s ", dict->super.name);
+		g_free (dict->super.name);
+		dict->super.name = tmp;
 
-	gboolean result = TRUE;
-	if ((*e = g_async_queue_try_pop (ctx.error_queue)))
-		result = FALSE;
-
-	g_async_queue_unref (ctx.error_queue);
-	return result;
+		gunichar *ucs4 = g_utf8_to_ucs4_fast (dict->super.name, -1, NULL);
+		for (gunichar *it = ucs4; *it; it++)
+			dict->name_width += app_char_width (self, *it);
+		g_free (ucs4);
+	}
+	return TRUE;
 }
 
-#define app_load_dictionaries app_load_dictionaries_parallel
-#endif  // GLib >= 2.36
-
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 
 /// Initialize the application core.
@@ -623,9 +549,8 @@ app_init (Application *self, char **filenames)
 	self->focused = TRUE;
 
 	app_init_attrs (self);
-	self->dictionaries = g_array_new (FALSE, TRUE, sizeof (Dictionary));
-	g_array_set_clear_func
-		(self->dictionaries, (GDestroyNotify) dictionary_free);
+	self->dictionaries =
+		g_ptr_array_new_with_free_func ((GDestroyNotify) dictionary_destroy);
 
 	GError *error = NULL;
 	app_load_config (self, &error);
@@ -640,11 +565,12 @@ app_init (Application *self, char **filenames)
 	// Dictionaries given on the command line override the configuration
 	if (*filenames)
 	{
-		g_array_set_size (self->dictionaries, 0);
+		g_ptr_array_set_size (self->dictionaries, 0);
 		while (*filenames)
 		{
-			Dictionary dict = { .filename = g_strdup (*filenames++) };
-			g_array_append_val (self->dictionaries, dict);
+			AppDictionary *dict = g_malloc0 (sizeof *dict);
+			dict->super.filename = g_strdup (*filenames++);
+			g_ptr_array_add (self->dictionaries, dict);
 		}
 	}
 
@@ -659,7 +585,8 @@ app_init (Application *self, char **filenames)
 			"the configuration or on the command line"));
 		exit (EXIT_FAILURE);
 	}
-	self->dict = g_array_index (self->dictionaries, Dictionary, 0).dict;
+	self->dict = ((AppDictionary *)
+		g_ptr_array_index (self->dictionaries, 0))->super.dict;
 	app_reload_view (self);
 }
 
@@ -715,7 +642,7 @@ app_destroy (Application *self)
 	g_ptr_array_free (self->entries, TRUE);
 	g_free (self->search_label);
 	g_array_free (self->input, TRUE);
-	g_array_free (self->dictionaries, TRUE);
+	g_ptr_array_free (self->dictionaries, TRUE);
 
 	g_iconv_close (self->ucs4_to_locale);
 }
@@ -935,7 +862,7 @@ app_redraw_top (Application *self)
 
 	for (guint i = 0; i < self->dictionaries->len; i++)
 	{
-		Dictionary *dict = &g_array_index (self->dictionaries, Dictionary, i);
+		Dictionary *dict = g_ptr_array_index (self->dictionaries, i);
 		row_buffer_append (&buf, dict->name,
 			APP_ATTR_IF (self->dictionaries->len > 1
 				&& self->dict == dict->dict, ACTIVE, HEADER));
@@ -1410,7 +1337,7 @@ app_goto_dictionary (Application *self, guint n)
 	if (n >= self->dictionaries->len)
 		return FALSE;
 
-	Dictionary *dict = &g_array_index (self->dictionaries, Dictionary, n);
+	Dictionary *dict = g_ptr_array_index (self->dictionaries, n);
 	self->dict = dict->dict;
 	app_search_for_entry (self);
 	app_redraw_top (self);
@@ -1421,13 +1348,13 @@ app_goto_dictionary (Application *self, guint n)
 static gboolean
 app_goto_dictionary_delta (Application *self, gint n)
 {
-	GArray *dicts = self->dictionaries;
+	GPtrArray *dicts = self->dictionaries;
 	if (dicts->len <= 1)
 		return FALSE;
 
 	guint i = 0;
 	while (i < dicts->len &&
-		g_array_index (dicts, Dictionary, i).dict != self->dict)
+		((Dictionary *) g_ptr_array_index (dicts, i))->dict != self->dict)
 		i++;
 
 	return app_goto_dictionary (self, (i + dicts->len + n) % dicts->len);
@@ -1858,10 +1785,10 @@ app_process_left_mouse_click (Application *self, int line, int column)
 		if (column < indent)
 			return;
 
-		Dictionary *dicts = (Dictionary *) self->dictionaries->data;
 		for (guint i = 0; i < self->dictionaries->len; i++)
 		{
-			if (column < (indent += dicts[i].name_width))
+			AppDictionary *dict = g_ptr_array_index (self->dictionaries, i);
+			if (column < (indent += dict->name_width))
 			{
 				app_goto_dictionary (self, i);
 				return;

src/utils.c

@@ -113,7 +113,7 @@ fatal (const gchar *format, ...)
 // At times, GLib even with its sheer size is surprisingly useless,
 // and I need to port some code over from "liberty".
 
-static gchar **
+static const gchar **
 get_xdg_config_dirs (void)
 {
 	GPtrArray *paths = g_ptr_array_new ();
@@ -122,12 +122,12 @@ get_xdg_config_dirs (void)
 		*system; system++)
 		g_ptr_array_add (paths, (gpointer) *system);
 	g_ptr_array_add (paths, NULL);
-	return (gchar **) g_ptr_array_free (paths, FALSE);
+	return (const gchar **) g_ptr_array_free (paths, FALSE);
 }
 
 gchar *
 resolve_relative_filename_generic
-	(gchar **paths, const gchar *tail, const gchar *filename)
+	(const gchar **paths, const gchar *tail, const gchar *filename)
 {
 	for (; *paths; paths++)
 	{
@@ -147,10 +147,10 @@ resolve_relative_filename_generic
 gchar *
 resolve_relative_config_filename (const gchar *filename)
 {
-	gchar **paths = get_xdg_config_dirs ();
-	gchar *result = resolve_relative_filename_generic
-		(paths, PROJECT_NAME, filename);
-	g_strfreev (paths);
+	const gchar **paths = get_xdg_config_dirs ();
+	gchar *result =
+		resolve_relative_filename_generic (paths, PROJECT_NAME, filename);
+	g_free (paths);
 	return result;
 }
 
@@ -202,7 +202,7 @@ GKeyFile *
 load_project_config_file (GError **error)
 {
 	GKeyFile *key_file = g_key_file_new ();
-	gchar **paths = get_xdg_config_dirs ();
+	const gchar **paths = get_xdg_config_dirs ();
 	GError *e = NULL;
 
 	// XXX: if there are dashes in the final path component,
@@ -210,8 +210,8 @@ load_project_config_file (GError **error)
 	//   which is completely undocumented
 	g_key_file_load_from_dirs (key_file,
 		PROJECT_NAME G_DIR_SEPARATOR_S PROJECT_NAME ".conf",
-		(const gchar **) paths, NULL, 0, &e);
-	g_strfreev (paths);
+		paths, NULL, 0, &e);
+	g_free (paths);
 	if (!e)
 		return key_file;
 
@@ -223,3 +223,84 @@ load_project_config_file (GError **error)
 	g_key_file_free (key_file);
 	return NULL;
 }
+
+// --- Loading -----------------------------------------------------------------
+
+void
+dictionary_destroy (Dictionary *self)
+{
+	g_free (self->name);
+	g_free (self->filename);
+
+	if (self->dict)
+		g_object_unref (self->dict);
+
+	g_free (self);
+}
+
+static gboolean
+dictionary_load (Dictionary *self, GError **e)
+{
+	if (!(self->dict = stardict_dict_new (self->filename, e)))
+		return FALSE;
+
+	if (!self->name)
+	{
+		self->name = g_strdup (stardict_info_get_book_name
+			(stardict_dict_get_info (self->dict)));
+	}
+	return TRUE;
+}
+
+static gboolean
+load_dictionaries_sequentially (GPtrArray *dictionaries, GError **e)
+{
+	for (guint i = 0; i < dictionaries->len; i++)
+		if (!dictionary_load (g_ptr_array_index (dictionaries, i), e))
+			return FALSE;
+	return TRUE;
+}
+
+// Parallelize dictionary loading if possible, because of collation reindexing
+#if GLIB_CHECK_VERSION (2, 36, 0)
+static void
+load_worker (gpointer data, gpointer user_data)
+{
+	GError *e = NULL;
+	dictionary_load (data, &e);
+	if (e)
+		g_async_queue_push (user_data, e);
+}
+
+gboolean
+load_dictionaries (GPtrArray *dictionaries, GError **e)
+{
+	GAsyncQueue *error_queue =
+		g_async_queue_new_full ((GDestroyNotify) g_error_free);
+	GThreadPool *pool = g_thread_pool_new (load_worker, error_queue,
+		g_get_num_processors (), TRUE, NULL);
+	if G_UNLIKELY (!g_thread_pool_get_num_threads (pool))
+	{
+		g_thread_pool_free (pool, TRUE, TRUE);
+		g_async_queue_unref (error_queue);
+		return load_dictionaries_sequentially (dictionaries, e);
+	}
+
+	for (guint i = 0; i < dictionaries->len; i++)
+		g_thread_pool_push (pool, g_ptr_array_index (dictionaries, i), NULL);
+	g_thread_pool_free (pool, FALSE, TRUE);
+
+	gboolean result = TRUE;
+	if ((*e = g_async_queue_try_pop (error_queue)))
+		result = FALSE;
+
+	g_async_queue_unref (error_queue);
+	return result;
+}
+#else  // GLib < 2.36
+gboolean
+load_dictionaries (GPtrArray *dictionaries, GError **e)
+{
+	return load_dictionaries_sequentially (dictionaries, e);
+}
+#endif  // GLib < 2.36

src/utils.h

@@ -19,6 +19,11 @@
 #ifndef UTILS_H
 #define UTILS_H
 
+#include <glib.h>
+#include <gio/gio.h>
+
+#include "stardict.h"
+
 /// After this statement, the element has been found and its index is stored
 /// in the variable "imid".
 #define BINARY_SEARCH_BEGIN(max, compare)                                     \
@@ -44,10 +49,24 @@ gboolean xstrtoul (unsigned long *out, const char *s, int base);
 void fatal (const gchar *format, ...) G_GNUC_PRINTF (1, 2) G_GNUC_NORETURN;
 
 gchar *resolve_relative_filename_generic
-	(gchar **paths, const gchar *tail, const gchar *filename);
+	(const gchar **paths, const gchar *tail, const gchar *filename);
 gchar *resolve_relative_config_filename (const gchar *filename);
 gchar *resolve_filename
 	(const gchar *filename, gchar *(*relative_cb) (const char *));
 GKeyFile *load_project_config_file (GError **error);
 
+// --- Loading -----------------------------------------------------------------
+
+typedef struct dictionary Dictionary;
+
+struct dictionary
+{
+	gchar        *filename;          ///< Path to the dictionary
+	StardictDict *dict;              ///< StarDict dictionary data
+	gchar        *name;              ///< Name to show
+};
+
+void dictionary_destroy (Dictionary *self);
+gboolean load_dictionaries (GPtrArray *dictionaries, GError **e);
+
 #endif  // ! UTILS_H