From c5c4b2cafb32489c974850462a8c130a379f40c3 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Fri, 6 Jul 2018 19:39:13 +0200 Subject: [PATCH 01/12] Use native notification functions in test Since we'll allocate also some recursive elements with notification_create, we have to free the notification properly. --- test/notification.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/test/notification.c b/test/notification.c index de189ad..b2673ac 100644 --- a/test/notification.c +++ b/test/notification.c @@ -104,21 +104,25 @@ SUITE(suite_notification) load_settings("data/dunstrc.default"); struct notification *a = notification_create(); - a->appname = "Test"; - a->summary = "Summary"; - a->body = "Body"; - a->icon = "Icon"; + a->appname = g_strdup("Test"); + a->summary = g_strdup("Summary"); + a->body = g_strdup("Body"); + a->icon = g_strdup("Icon"); a->urgency = URG_NORM; struct notification *b = notification_create(); - memcpy(b, a, sizeof(*b)); + b->appname = g_strdup("Test"); + b->summary = g_strdup("Summary"); + b->body = g_strdup("Body"); + b->icon = g_strdup("Icon"); + b->urgency = URG_NORM; //2 equal notifications to be passed for duplicate checking, struct notification *n[2] = {a, b}; RUN_TEST1(test_notification_is_duplicate, (void*) n); - g_free(a); - g_free(b); + notification_free(a); + notification_free(b); RUN_TEST(test_notification_replace_single_field); From 837b4fe1251f2ac8cf978b1b4ef27afc2c85269d Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Fri, 6 Jul 2018 19:41:02 +0200 Subject: [PATCH 02/12] Implement refcounting for notifications --- src/dbus.c | 2 +- src/notification.c | 34 +++++++++++++++++++++++++++++++++- src/notification.h | 16 +++++++++++++--- src/queues.c | 10 +++++----- test/notification.c | 4 ++-- 5 files changed, 54 insertions(+), 12 deletions(-) diff --git a/src/dbus.c b/src/dbus.c index fb75b75..603a510 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -282,7 +282,7 @@ static void on_notify(GDBusConnection *connection, // The message got discarded if (id == 0) { signal_notification_closed(n, 2); - notification_free(n); + notification_unref(n); } wake_up(); diff --git a/src/notification.c b/src/notification.c index b36a817..906deab 100644 --- a/src/notification.c +++ b/src/notification.c @@ -42,6 +42,10 @@ const char *enum_to_string_fullscreen(enum behavior_fullscreen in) } } +struct _notification_private { + gint refcount; +}; + /* see notification.h */ void notification_print(const struct notification *n) { @@ -206,12 +210,28 @@ void rawimage_free(struct raw_image *i) g_free(i); } +static void notification_private_free(NotificationPrivate *p) +{ + g_free(p); +} + /* see notification.h */ -void notification_free(struct notification *n) +void notification_ref(struct notification *n) +{ + assert(n->priv->refcount > 0); + g_atomic_int_inc(&n->priv->refcount); +} + +/* see notification.h */ +void notification_unref(struct notification *n) { if (!n) return; + assert(n->priv->refcount > 0); + if (!g_atomic_int_dec_and_test(&n->priv->refcount)) + return; + g_free(n->appname); g_free(n->summary); g_free(n->body); @@ -228,6 +248,8 @@ void notification_free(struct notification *n) actions_free(n->actions); rawimage_free(n->raw_icon); + notification_private_free(n->priv); + g_free(n); } @@ -255,11 +277,21 @@ void notification_replace_single_field(char **haystack, g_free(input); } +static NotificationPrivate *notification_private_create(void) +{ + NotificationPrivate *priv = g_malloc0(sizeof(NotificationPrivate)); + g_atomic_int_set(&priv->refcount, 1); + + return priv; +} + /* see notification.h */ struct notification *notification_create(void) { struct notification *n = g_malloc0(sizeof(struct notification)); + n->priv = notification_private_create(); + /* Unparameterized default values */ n->first_render = true; n->markup = settings.markup; diff --git a/src/notification.h b/src/notification.h index 80d64ab..6f9731b 100644 --- a/src/notification.h +++ b/src/notification.h @@ -42,7 +42,10 @@ struct actions { gsize count; }; +typedef struct _notification_private NotificationPrivate; + struct notification { + NotificationPrivate *priv; int id; char *dbus_client; @@ -90,11 +93,18 @@ struct notification { * - the default (if it's not needed to be freed later) * - its undefined representation (NULL, -1) * + * The reference counter is set to 1. + * * This function is guaranteed to return a valid pointer. * @returns The generated notification */ struct notification *notification_create(void); +/** + * Increase the reference counter of the notification. + */ +void notification_ref(struct notification *n); + /** * Sanitize values of notification, apply all matching rules * and generate derived fields. @@ -118,11 +128,11 @@ void actions_free(struct actions *a); void rawimage_free(struct raw_image *i); /** - * Free the memory used by the given notification. + * Decrease the reference counter of the notification. * - * @param n (nullable): pointer to #notification + * If the reference count drops to 0, the object gets freed. */ -void notification_free(struct notification *n); +void notification_unref(struct notification *n); /** * Helper function to compare two given notifications. diff --git a/src/queues.c b/src/queues.c index f5d2077..06efc50 100644 --- a/src/queues.c +++ b/src/queues.c @@ -191,7 +191,7 @@ static bool queues_stack_duplicate(struct notification *n) if ( allqueues[i] == displayed ) n->start = time_monotonic_now(); - notification_free(orig); + notification_unref(orig); return true; } } @@ -218,7 +218,7 @@ bool queues_notification_replace_id(struct notification *new) notification_run_script(new); } - notification_free(old); + notification_unref(old); return true; } } @@ -278,12 +278,12 @@ void queues_history_push(struct notification *n) if (!n->history_ignore) { if (settings.history_length > 0 && history->length >= settings.history_length) { struct notification *to_free = g_queue_pop_head(history); - notification_free(to_free); + notification_unref(to_free); } g_queue_push_tail(history, n); } else { - notification_free(n); + notification_unref(n); } } @@ -488,7 +488,7 @@ bool queues_pause_status(void) static void teardown_notification(gpointer data) { struct notification *n = data; - notification_free(n); + notification_unref(n); } /* see queues.h */ diff --git a/test/notification.c b/test/notification.c index b2673ac..3a29e98 100644 --- a/test/notification.c +++ b/test/notification.c @@ -121,8 +121,8 @@ SUITE(suite_notification) struct notification *n[2] = {a, b}; RUN_TEST1(test_notification_is_duplicate, (void*) n); - notification_free(a); - notification_free(b); + notification_unref(a); + notification_unref(b); RUN_TEST(test_notification_replace_single_field); From 974bcb776ee6c5d2399a09befad3c133cea7891d Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Fri, 6 Jul 2018 20:03:43 +0200 Subject: [PATCH 03/12] Remember if the notification is living on a valid connection The notification spec says, that a notification gets invalidated when closed. So the client won't listen anymore to ActionInvoked signals and won't listen to NotificationClosed signals. Remembering the actual status of the notification helps the standard and makes the behavior clearer. --- src/dbus.c | 15 +++++++++++++++ src/notification.c | 1 + src/notification.h | 1 + 3 files changed, 17 insertions(+) diff --git a/src/dbus.c b/src/dbus.c index 603a510..c485a57 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -137,6 +137,7 @@ static struct notification *dbus_message_to_notification(const gchar *sender, GV n->actions = g_malloc0(sizeof(struct actions)); n->dbus_client = g_strdup(sender); + n->dbus_valid = true; { GVariantIter *iter = g_variant_iter_new(parameters); @@ -316,6 +317,12 @@ static void on_get_server_information(GDBusConnection *connection, void signal_notification_closed(struct notification *n, enum reason reason) { + if (!n->dbus_valid) { + LOG_W("Closing notification '%s' not supported. " + "Notification already closed.", n->summary); + return; + } + if (reason < REASON_MIN || REASON_MAX < reason) { LOG_W("Closing notification with reason '%d' not supported. " "Closing it with reason '%d'.", reason, REASON_UNDEF); @@ -337,6 +344,8 @@ void signal_notification_closed(struct notification *n, enum reason reason) body, &err); + n->dbus_valid = false; + if (err) { LOG_W("Unable to close notification: %s", err->message); g_error_free(err); @@ -346,6 +355,12 @@ void signal_notification_closed(struct notification *n, enum reason reason) void signal_action_invoked(const struct notification *n, const char *identifier) { + if (!n->dbus_valid) { + LOG_W("Invoking action '%s' not supported. " + "Notification already closed.", identifier); + return; + } + GVariant *body = g_variant_new("(us)", n->id, identifier); GError *err = NULL; diff --git a/src/notification.c b/src/notification.c index 906deab..c08fbe2 100644 --- a/src/notification.c +++ b/src/notification.c @@ -306,6 +306,7 @@ struct notification *notification_create(void) n->progress = -1; n->script_run = false; + n->dbus_valid = false; n->fullscreen = FS_SHOW; diff --git a/src/notification.h b/src/notification.h index 6f9731b..0764152 100644 --- a/src/notification.h +++ b/src/notification.h @@ -48,6 +48,7 @@ struct notification { NotificationPrivate *priv; int id; char *dbus_client; + bool dbus_valid; char *appname; char *summary; From 958aa2bc9658940c0f747049d91546e0832ff54a Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Fri, 6 Jul 2018 20:47:42 +0200 Subject: [PATCH 04/12] Lock notifications while executing dmenu When executing dmenu, the current notifications get "locked", by setting their timeout temporarily to 0 and referencing them. So the notification won't get closed (exept forcefully) and won't get freed while dmenu is opened. This is currently pointless, but as the dmenu call will become threaded, it's necessary later. --- src/menu.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/menu.c b/src/menu.c index d724cdd..33a4a93 100644 --- a/src/menu.c +++ b/src/menu.c @@ -23,6 +23,11 @@ static bool is_initialized = false; static regex_t cregex; +struct notification_lock { + struct notification *n; + gint64 timeout; +}; + static int regex_init(void) { if (is_initialized) @@ -199,10 +204,27 @@ void context_menu(void) } char *dmenu_input = NULL; + GList *locked_notifications = NULL; + for (const GList *iter = queues_get_displayed(); iter; iter = iter->next) { struct notification *n = iter->data; + + // Reference and lock the notification if we need it + if (n->urls || n->actions) { + notification_ref(n); + + struct notification_lock *nl = + g_malloc(sizeof(struct notification_lock)); + + nl->n = n; + nl->timeout = n->timeout; + n->timeout = 0; + + locked_notifications = g_list_prepend(locked_notifications, nl); + } + if (n->urls) dmenu_input = string_append(dmenu_input, n->urls, "\n"); @@ -272,5 +294,20 @@ void context_menu(void) dispatch_menu_result(buf); g_free(dmenu_input); + + // unref all notifications + for (GList *iter = locked_notifications; + iter; + iter = iter->next) { + + struct notification_lock *nl = iter->data; + struct notification *n = nl->n; + + n->timeout = nl->timeout; + + g_free(nl); + notification_unref(n); + } + g_list_free(locked_notifications); } /* vim: set tabstop=8 shiftwidth=8 expandtab textwidth=0: */ From 38c788c3674ca21eef39ede26520e6c59aafd997 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Sat, 7 Jul 2018 02:26:24 +0200 Subject: [PATCH 05/12] Split plain dmenu call into separate function --- src/menu.c | 102 +++++++++++++++++++++++++++++------------------------ 1 file changed, 56 insertions(+), 46 deletions(-) diff --git a/src/menu.c b/src/menu.c index 33a4a93..1aea9aa 100644 --- a/src/menu.c +++ b/src/menu.c @@ -192,63 +192,31 @@ void dispatch_menu_result(const char *input) g_free(in); } -/* - * Open the context menu that let's the user - * select urls/actions/etc +/** Call dmenu with the specified input. Blocks until dmenu is finished. + * + * @param dmenu_input The input string to feed into dmenu + * @returns the selected string from dmenu */ -void context_menu(void) +char *invoke_dmenu(const char *dmenu_input) { if (!settings.dmenu_cmd) { LOG_C("Unable to open dmenu: No dmenu command set."); - return; - } - char *dmenu_input = NULL; - - GList *locked_notifications = NULL; - - for (const GList *iter = queues_get_displayed(); iter; - iter = iter->next) { - struct notification *n = iter->data; - - - // Reference and lock the notification if we need it - if (n->urls || n->actions) { - notification_ref(n); - - struct notification_lock *nl = - g_malloc(sizeof(struct notification_lock)); - - nl->n = n; - nl->timeout = n->timeout; - n->timeout = 0; - - locked_notifications = g_list_prepend(locked_notifications, nl); - } - - if (n->urls) - dmenu_input = string_append(dmenu_input, n->urls, "\n"); - - if (n->actions) - dmenu_input = - string_append(dmenu_input, n->actions->dmenu_str, - "\n"); + return NULL; } - if (!dmenu_input) - return; + if (!dmenu_input || *dmenu_input == '\0') + return NULL; char buf[1024] = {0}; int child_io[2]; int parent_io[2]; if (pipe(child_io) != 0) { LOG_W("pipe(): error in child: %s", strerror(errno)); - g_free(dmenu_input); - return; + return NULL; } if (pipe(parent_io) != 0) { LOG_W("pipe(): error in parent: %s", strerror(errno)); - g_free(dmenu_input); - return; + return NULL; } int pid = fork(); @@ -284,16 +252,58 @@ void context_menu(void) waitpid(pid, NULL, 0); if (len == 0) { - g_free(dmenu_input); - return; + return NULL; } } - close(parent_io[0]); - dispatch_menu_result(buf); + return g_strdup(buf); +} + +/* + * Open the context menu that let's the user + * select urls/actions/etc + */ +void context_menu(void) +{ + char *dmenu_input = NULL; + char *dmenu_output; + + GList *locked_notifications = NULL; + + for (const GList *iter = queues_get_displayed(); iter; + iter = iter->next) { + struct notification *n = iter->data; + + + // Reference and lock the notification if we need it + if (n->urls || n->actions) { + notification_ref(n); + + struct notification_lock *nl = + g_malloc(sizeof(struct notification_lock)); + + nl->n = n; + nl->timeout = n->timeout; + n->timeout = 0; + + locked_notifications = g_list_prepend(locked_notifications, nl); + } + + if (n->urls) + dmenu_input = string_append(dmenu_input, n->urls, "\n"); + + if (n->actions) + dmenu_input = + string_append(dmenu_input, n->actions->dmenu_str, + "\n"); + } + + dmenu_output = invoke_dmenu(dmenu_input); + dispatch_menu_result(dmenu_output); g_free(dmenu_input); + g_free(dmenu_output); // unref all notifications for (GList *iter = locked_notifications; From c80e3e9a42f71d545293f32077ada41522cc08d2 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Sat, 7 Jul 2018 11:13:14 +0200 Subject: [PATCH 06/12] Harness dispatch_menu_result against stupid input --- src/menu.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/menu.c b/src/menu.c index 1aea9aa..9f82afa 100644 --- a/src/menu.c +++ b/src/menu.c @@ -176,19 +176,25 @@ void invoke_action(const char *action) } } -/* - * Dispatch whatever has been returned - * by the menu. +/** + * Dispatch whatever has been returned by dmenu. + * If the given result of dmenu is empty or NULL, nothing will be done. + * + * @param input The result from dmenu. */ void dispatch_menu_result(const char *input) { + if (!input) + return; + char *in = g_strdup(input); g_strstrip(in); - if (in[0] == '#') { + + if (in[0] == '#') invoke_action(in + 1); - } else { + else if (in[0] != '\0') open_browser(in); - } + g_free(in); } From 851953f5ef7ffa1a0d14808c55461c05df32de71 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Sat, 7 Jul 2018 11:21:33 +0200 Subject: [PATCH 07/12] Start dmenu in a separate thread --- src/menu.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/menu.c b/src/menu.c index 9f82afa..24626db 100644 --- a/src/menu.c +++ b/src/menu.c @@ -27,6 +27,7 @@ struct notification_lock { struct notification *n; gint64 timeout; }; +static gpointer context_menu_thread(gpointer data); static int regex_init(void) { @@ -271,6 +272,20 @@ char *invoke_dmenu(const char *dmenu_input) * select urls/actions/etc */ void context_menu(void) +{ + GError *err = NULL; + g_thread_unref(g_thread_try_new("dmenu", + context_menu_thread, + NULL, + &err)); + + if (err) { + LOG_C("Cannot start thread to call dmenu: %s", err->message); + g_error_free(err); + } +} + +static gpointer context_menu_thread(gpointer data) { char *dmenu_input = NULL; char *dmenu_output; @@ -325,5 +340,7 @@ void context_menu(void) notification_unref(n); } g_list_free(locked_notifications); + + return NULL; } /* vim: set tabstop=8 shiftwidth=8 expandtab textwidth=0: */ From 357c4309e651fe8a78c540ceb508917d969f4f34 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Wed, 26 Sep 2018 09:36:24 +0200 Subject: [PATCH 08/12] Pass URLs to browser as a single argument Parsing the arguments with g_shell_parse_argv is more safe than just splitting it by spaces. Also this allows to pass values with spaces to the browser command. --- src/menu.c | 18 +++++++++++++++--- src/settings.c | 10 ++++++++++ src/settings.h | 1 + 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/menu.c b/src/menu.c index 24626db..4156aad 100644 --- a/src/menu.c +++ b/src/menu.c @@ -102,6 +102,11 @@ char *extract_urls(const char *to_match) */ void open_browser(const char *in) { + if (!settings.browser_cmd) { + LOG_C("Unable to open browser: No browser command set."); + return; + } + char *url = NULL; // If any, remove leading [ linktext ] from URL @@ -122,9 +127,16 @@ void open_browser(const char *in) if (browser_pid2) { exit(0); } else { - char *browser_cmd = g_strconcat(settings.browser, " ", url, NULL); - char **cmd = g_strsplit(browser_cmd, " ", 0); - execvp(cmd[0], cmd); + int argc = 2+g_strv_length(settings.browser_cmd); + char **argv = g_malloc_n(argc, sizeof(char*)); + + memcpy(argv, settings.browser_cmd, argc * sizeof(char*)); + argv[argc-2] = url; + argv[argc-1] = NULL; + + execvp(argv[0], argv); + g_free(argv); + // execvp won't return if it's successful // so, if we're here, it's definitely an error fprintf(stderr, "Warning: failed to execute '%s': %s\n", diff --git a/src/settings.c b/src/settings.c index 110b98b..411f85e 100644 --- a/src/settings.c +++ b/src/settings.c @@ -448,6 +448,16 @@ void load_settings(char *cmdline_config_path) "path to browser" ); + { + GError *error = NULL; + if (!g_shell_parse_argv(settings.browser, NULL, &settings.browser_cmd, &error)) { + LOG_W("Unable to parse browser command: '%s'." + " URL functionality will be disabled.", error->message); + g_error_free(error); + settings.browser_cmd = NULL; + } + } + { char *c = option_get_string( "global", diff --git a/src/settings.h b/src/settings.h index afa39c1..21419e0 100644 --- a/src/settings.h +++ b/src/settings.h @@ -75,6 +75,7 @@ struct settings { char *dmenu; char **dmenu_cmd; char *browser; + char **browser_cmd; enum icon_position icon_position; int max_icon_size; char *icon_path; From 6cc7ca361aae2b98c3ab6d7ec2b650c425ffbc4e Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Mon, 1 Oct 2018 09:38:37 +0200 Subject: [PATCH 09/12] Allow half quoted values Previously config lines like [rule] script = mail -s "New notif" were only possible to get written with additional full quotes, which makes no sense in command line expressions. --- src/option_parser.c | 8 +++----- test/data/test-ini | 1 + test/option_parser.c | 2 ++ 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/option_parser.c b/src/option_parser.c index 0cb8354..16fe623 100644 --- a/src/option_parser.c +++ b/src/option_parser.c @@ -203,16 +203,14 @@ int ini_get_bool(const char *section, const char *key, int def) char *clean_value(const char *value) { + size_t len = strlen(value); char *s; - if (value[0] == '"') - s = g_strdup(value + 1); + if (value[0] == '"' && value[len-1] == '"') + s = g_strndup(value + 1, len-2); else s = g_strdup(value); - if (s[strlen(s) - 1] == '"') - s[strlen(s) - 1] = '\0'; - return s; } diff --git a/test/data/test-ini b/test/data/test-ini index 9d2f7cd..0dab3e3 100644 --- a/test/data/test-ini +++ b/test/data/test-ini @@ -22,6 +22,7 @@ simple = A simple string quoted = "A quoted string" quoted_with_quotes = "A string "with quotes"" + unquoted_with_quotes = A" string with quotes" [path] expand_tilde = ~/.path/to/tilde diff --git a/test/option_parser.c b/test/option_parser.c index f2d0960..3b942d8 100644 --- a/test/option_parser.c +++ b/test/option_parser.c @@ -53,6 +53,8 @@ TEST test_ini_get_string(void) free(ptr); ASSERT_STR_EQ("A string \"with quotes\"", (ptr = ini_get_string(string_section, "quoted_with_quotes", ""))); free(ptr); + ASSERT_STR_EQ("A\" string with quotes\"", (ptr = ini_get_string(string_section, "unquoted_with_quotes", ""))); + free(ptr); ASSERT_STR_EQ("default value", (ptr = ini_get_string(string_section, "nonexistent", "default value"))); free(ptr); From 778a6857d87205350f0c91f83843a6d6b713dd53 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Mon, 1 Oct 2018 09:54:16 +0200 Subject: [PATCH 10/12] Move clean_value function to utils.c This is a typical string manipulation function an has no necessity to stay in option_parser. --- src/option_parser.c | 16 +--------------- src/utils.c | 17 +++++++++++++++++ src/utils.h | 8 ++++++++ test/utils.c | 27 +++++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 15 deletions(-) diff --git a/src/option_parser.c b/src/option_parser.c index 16fe623..849a2ba 100644 --- a/src/option_parser.c +++ b/src/option_parser.c @@ -30,7 +30,6 @@ static struct section *new_section(const char *name); static struct section *get_section(const char *name); static void add_entry(const char *section_name, const char *key, const char *value); static const char *get_value(const char *section, const char *key); -static char *clean_value(const char *value); static int cmdline_argc; static char **cmdline_argv; @@ -90,7 +89,7 @@ void add_entry(const char *section_name, const char *key, const char *value) int len = s->entry_count; s->entries = g_realloc(s->entries, sizeof(struct entry) * len); s->entries[s->entry_count - 1].key = g_strdup(key); - s->entries[s->entry_count - 1].value = clean_value(value); + s->entries[s->entry_count - 1].value = string_strip_quotes(value); } const char *get_value(const char *section, const char *key) @@ -201,19 +200,6 @@ int ini_get_bool(const char *section, const char *key, int def) } } -char *clean_value(const char *value) -{ - size_t len = strlen(value); - char *s; - - if (value[0] == '"' && value[len-1] == '"') - s = g_strndup(value + 1, len-2); - else - s = g_strdup(value); - - return s; -} - int load_ini_file(FILE *fp) { if (!fp) diff --git a/src/utils.c b/src/utils.c index 8f06141..e5e7d54 100644 --- a/src/utils.c +++ b/src/utils.c @@ -97,6 +97,23 @@ char *string_append(char *a, const char *b, const char *sep) } +/* see utils.h */ +char *string_strip_quotes(const char *value) +{ + if (!value) + return NULL; + + size_t len = strlen(value); + char *s; + + if (value[0] == '"' && value[len-1] == '"') + s = g_strndup(value + 1, len-2); + else + s = g_strdup(value); + + return s; +} + void string_strip_delimited(char *str, char a, char b) { int iread=-1, iwrite=0, copen=0; diff --git a/src/utils.h b/src/utils.h index 48ee618..ca9022e 100644 --- a/src/utils.h +++ b/src/utils.h @@ -21,6 +21,14 @@ char *string_append(char *a, const char *b, const char *sep); /* strip content between two delimiter characters (inplace) */ void string_strip_delimited(char *str, char a, char b); +/** + * Strip quotes from a string. Won't touch inner quotes. + * + * @param value The string to strip the quotes from + * @returns A copy of the string value with the outer quotes removed (if any) + */ +char *string_strip_quotes(const char *value); + /* replace tilde and path-specific values with its equivalents */ char *string_to_path(char *string); diff --git a/test/utils.c b/test/utils.c index d0b5f4b..b99af9e 100644 --- a/test/utils.c +++ b/test/utils.c @@ -104,6 +104,32 @@ TEST test_string_append(void) PASS(); } +TEST test_string_strip_quotes(void) +{ + char *exp = string_strip_quotes(NULL); + ASSERT_FALSE(exp); + + ASSERT_STR_EQ("NewString", (exp = string_strip_quotes("NewString"))); + g_free(exp); + + ASSERT_STR_EQ("becomes unquoted", (exp = string_strip_quotes("\"becomes unquoted\""))); + g_free(exp); + + ASSERT_STR_EQ("\"stays quoted", (exp = string_strip_quotes("\"stays quoted"))); + g_free(exp); + + ASSERT_STR_EQ("stays quoted\"", (exp = string_strip_quotes("stays quoted\""))); + g_free(exp); + + ASSERT_STR_EQ("stays \"quoted\"", (exp = string_strip_quotes("stays \"quoted\""))); + g_free(exp); + + ASSERT_STR_EQ(" \"stays quoted\"", (exp = string_strip_quotes(" \"stays quoted\""))); + g_free(exp); + + PASS(); +} + TEST test_string_strip_delimited(void) { char *text = malloc(128 * sizeof(char)); @@ -178,6 +204,7 @@ SUITE(suite_utils) RUN_TEST(test_string_replace_all); RUN_TEST(test_string_replace); RUN_TEST(test_string_append); + RUN_TEST(test_string_strip_quotes); RUN_TEST(test_string_strip_delimited); RUN_TEST(test_string_to_path); RUN_TEST(test_string_to_time); From fe7d82380effa68313d01532d996f753fea515d7 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Fri, 5 Oct 2018 22:59:56 +0200 Subject: [PATCH 11/12] Use GLib to spawn browser and dmenu --- src/menu.c | 128 +++++++++++++++++++++++------------------------------ 1 file changed, 56 insertions(+), 72 deletions(-) diff --git a/src/menu.c b/src/menu.c index 4156aad..c45abd1 100644 --- a/src/menu.c +++ b/src/menu.c @@ -107,44 +107,40 @@ void open_browser(const char *in) return; } - char *url = NULL; - + char *url, *end; // If any, remove leading [ linktext ] from URL - const char *end = strstr(in, "] "); - if (*in == '[' && end) + if (*in == '[' && (end = strstr(in, "] "))) url = g_strdup(end + 2); else url = g_strdup(in); - int browser_pid1 = fork(); + int argc = 2+g_strv_length(settings.browser_cmd); + char **argv = g_malloc_n(argc, sizeof(char*)); - if (browser_pid1) { - g_free(url); - int status; - waitpid(browser_pid1, &status, 0); - } else { - int browser_pid2 = fork(); - if (browser_pid2) { - exit(0); - } else { - int argc = 2+g_strv_length(settings.browser_cmd); - char **argv = g_malloc_n(argc, sizeof(char*)); + memcpy(argv, settings.browser_cmd, argc * sizeof(char*)); + argv[argc-2] = url; + argv[argc-1] = NULL; - memcpy(argv, settings.browser_cmd, argc * sizeof(char*)); - argv[argc-2] = url; - argv[argc-1] = NULL; + GError *err = NULL; + g_spawn_async(NULL, + argv, + NULL, + G_SPAWN_DEFAULT + | G_SPAWN_SEARCH_PATH + | G_SPAWN_STDOUT_TO_DEV_NULL + | G_SPAWN_STDERR_TO_DEV_NULL, + NULL, + NULL, + NULL, + &err); - execvp(argv[0], argv); - g_free(argv); - - // execvp won't return if it's successful - // so, if we're here, it's definitely an error - fprintf(stderr, "Warning: failed to execute '%s': %s\n", - settings.browser, - strerror(errno)); - exit(EXIT_FAILURE); - } + if (err) { + LOG_C("Cannot spawn browser: %s", err->message); + g_error_free(err); } + + g_free(argv); + g_free(url); } /* @@ -226,57 +222,45 @@ char *invoke_dmenu(const char *dmenu_input) if (!dmenu_input || *dmenu_input == '\0') return NULL; - char buf[1024] = {0}; - int child_io[2]; - int parent_io[2]; - if (pipe(child_io) != 0) { - LOG_W("pipe(): error in child: %s", strerror(errno)); - return NULL; - } - if (pipe(parent_io) != 0) { - LOG_W("pipe(): error in parent: %s", strerror(errno)); - return NULL; - } - int pid = fork(); + gint dunst_to_dmenu; + gint dmenu_to_dunst; + GError *err = NULL; + char buf[1024]; + char *ret = NULL; - if (pid == 0) { - close(child_io[1]); - close(parent_io[0]); - close(0); - if (dup(child_io[0]) == -1) { - LOG_W("dup(): error in child: %s", strerror(errno)); - exit(EXIT_FAILURE); - } - close(1); - if (dup(parent_io[1]) == -1) { - LOG_W("dup(): error in parent: %s", strerror(errno)); - exit(EXIT_FAILURE); - } - execvp(settings.dmenu_cmd[0], settings.dmenu_cmd); - fprintf(stderr, "Warning: failed to execute '%s': %s\n", - settings.dmenu, - strerror(errno)); - exit(EXIT_FAILURE); + g_spawn_async_with_pipes(NULL, + settings.dmenu_cmd, + NULL, + G_SPAWN_DEFAULT + | G_SPAWN_SEARCH_PATH, + NULL, + NULL, + NULL, + &dunst_to_dmenu, + &dmenu_to_dunst, + NULL, + &err); + + if (err) { + LOG_C("Cannot spawn dmenu: %s", err->message); + g_error_free(err); } else { - close(child_io[0]); - close(parent_io[1]); size_t wlen = strlen(dmenu_input); - if (write(child_io[1], dmenu_input, wlen) != wlen) { - LOG_W("write(): error: %s", strerror(errno)); + if (write(dunst_to_dmenu, dmenu_input, wlen) != wlen) { + LOG_W("Cannot feed dmenu with input: %s", strerror(errno)); } - close(child_io[1]); + close(dunst_to_dmenu); - size_t len = read(parent_io[0], buf, 1023); + ssize_t rlen = read(dmenu_to_dunst, buf, sizeof(buf)); + close(dmenu_to_dunst); - waitpid(pid, NULL, 0); - - if (len == 0) { - return NULL; - } + if (rlen > 0) + ret = g_strndup(buf, rlen); + else + LOG_W("Didn't receive input from dmenu."); } - close(parent_io[0]); - return g_strdup(buf); + return ret; } /* From 52c47524f28160706658a02cc8b8ea32dcbf06c8 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Sat, 6 Oct 2018 08:36:05 +0200 Subject: [PATCH 12/12] Test notification referencing --- src/notification.c | 7 +++++++ src/notification.h | 5 +++++ test/notification.c | 19 +++++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/src/notification.c b/src/notification.c index c08fbe2..f4c6abf 100644 --- a/src/notification.c +++ b/src/notification.c @@ -215,6 +215,13 @@ static void notification_private_free(NotificationPrivate *p) g_free(p); } +/* see notification.h */ +gint notification_refcount_get(struct notification *n) +{ + assert(n->priv->refcount > 0); + return g_atomic_int_get(&n->priv->refcount); +} + /* see notification.h */ void notification_ref(struct notification *n) { diff --git a/src/notification.h b/src/notification.h index 0764152..ffc887b 100644 --- a/src/notification.h +++ b/src/notification.h @@ -101,6 +101,11 @@ struct notification { */ struct notification *notification_create(void); +/** + * Retrieve the current reference count of the notification + */ +gint notification_refcount_get(struct notification *n); + /** * Increase the reference counter of the notification. */ diff --git a/test/notification.c b/test/notification.c index 3a29e98..b6afe2f 100644 --- a/test/notification.c +++ b/test/notification.c @@ -98,6 +98,24 @@ TEST test_notification_replace_single_field(void) PASS(); } +TEST test_notification_referencing(void) +{ + struct notification *n = notification_create(); + ASSERT(notification_refcount_get(n) == 1); + + notification_ref(n); + ASSERT(notification_refcount_get(n) == 2); + + notification_unref(n); + ASSERT(notification_refcount_get(n) == 1); + + // Now we have to rely on valgrind to test, that + // it gets actually freed + notification_unref(n); + + PASS(); +} + SUITE(suite_notification) { cmdline_load(0, NULL); @@ -125,6 +143,7 @@ SUITE(suite_notification) notification_unref(b); RUN_TEST(test_notification_replace_single_field); + RUN_TEST(test_notification_referencing); g_clear_pointer(&settings.icon_path, g_free); }