From c0a094e4734a1a760681e75565af10bf86b68bee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emysl=20Eric=20Janouch?=
Date: Sat, 16 Oct 2021 08:27:42 +0200
Subject: [PATCH] sdgui: load dictionaries in parallel, as sdtui did
Also, resolve some use-after-frees in GTK+.
---
src/sdgui.c | 76 +++++++++-----------------
src/sdtui.c | 155 ++++++++++++++--------------------------------------
src/utils.c | 101 ++++++++++++++++++++++++++++++----
src/utils.h | 21 ++++++-
4 files changed, 178 insertions(+), 175 deletions(-)
diff --git a/src/sdgui.c b/src/sdgui.c
index cc2a042..b1b1329 100644
--- a/src/sdgui.c
+++ b/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;
}
diff --git a/src/sdtui.c b/src/sdtui.c
index caf08e6..f7cdb02 100644
--- a/src/sdtui.c
+++ b/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;
+ if (!load_dictionaries (self->dictionaries, e))
+ return FALSE;
+
+ for (gsize i = 0; i < self->dictionaries->len; i++)
+ {
+ AppDictionary *dict = g_ptr_array_index (self->dictionaries, i);
+
+ // Add some padding for decorative purposes
+ gchar *tmp = g_strdup_printf (" %s ", dict->super.name);
+ g_free (dict->super.name);
+ dict->super.name = tmp;
+
+ 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;
}
-// 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);
-}
-
-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))
- {
- 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);
-
- g_thread_pool_free (pool, FALSE, TRUE);
-
- gboolean result = TRUE;
- if ((*e = g_async_queue_try_pop (ctx.error_queue)))
- result = FALSE;
-
- g_async_queue_unref (ctx.error_queue);
- return result;
-}
-
-#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;
diff --git a/src/utils.c b/src/utils.c
index 612eaaa..367c0f5 100644
--- a/src/utils.c
+++ b/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
diff --git a/src/utils.h b/src/utils.h
index 1394f08..bc5775b 100644
--- a/src/utils.h
+++ b/src/utils.h
@@ -19,6 +19,11 @@
#ifndef UTILS_H
#define UTILS_H
+#include