diff --git a/src/dbus.c b/src/dbus.c index fb75b75..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); @@ -282,7 +283,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(); @@ -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/menu.c b/src/menu.c index d724cdd..c45abd1 100644 --- a/src/menu.c +++ b/src/menu.c @@ -23,6 +23,12 @@ static bool is_initialized = false; static regex_t cregex; +struct notification_lock { + struct notification *n; + gint64 timeout; +}; +static gpointer context_menu_thread(gpointer data); + static int regex_init(void) { if (is_initialized) @@ -96,37 +102,45 @@ char *extract_urls(const char *to_match) */ void open_browser(const char *in) { - char *url = NULL; + if (!settings.browser_cmd) { + LOG_C("Unable to open browser: No browser command set."); + return; + } + 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 { - char *browser_cmd = g_strconcat(settings.browser, " ", url, NULL); - char **cmd = g_strsplit(browser_cmd, " ", 0); - execvp(cmd[0], cmd); - // 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); - } + 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); + + if (err) { + LOG_C("Cannot spawn browser: %s", err->message); + g_error_free(err); } + + g_free(argv); + g_free(url); } /* @@ -171,38 +185,128 @@ 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); } +/** 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 + */ +char *invoke_dmenu(const char *dmenu_input) +{ + if (!settings.dmenu_cmd) { + LOG_C("Unable to open dmenu: No dmenu command set."); + return NULL; + } + + if (!dmenu_input || *dmenu_input == '\0') + return NULL; + + gint dunst_to_dmenu; + gint dmenu_to_dunst; + GError *err = NULL; + char buf[1024]; + char *ret = NULL; + + 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 { + size_t wlen = strlen(dmenu_input); + if (write(dunst_to_dmenu, dmenu_input, wlen) != wlen) { + LOG_W("Cannot feed dmenu with input: %s", strerror(errno)); + } + close(dunst_to_dmenu); + + ssize_t rlen = read(dmenu_to_dunst, buf, sizeof(buf)); + close(dmenu_to_dunst); + + if (rlen > 0) + ret = g_strndup(buf, rlen); + else + LOG_W("Didn't receive input from dmenu."); + } + + return ret; +} + /* * Open the context menu that let's the user * select urls/actions/etc */ void context_menu(void) { - if (!settings.dmenu_cmd) { - LOG_C("Unable to open dmenu: No dmenu command set."); - return; + 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; + + 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"); @@ -212,65 +316,27 @@ void context_menu(void) "\n"); } - if (!dmenu_input) - return; - - 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; - } - if (pipe(parent_io) != 0) { - LOG_W("pipe(): error in parent: %s", strerror(errno)); - g_free(dmenu_input); - return; - } - int pid = fork(); - - 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); - } 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)); - } - close(child_io[1]); - - size_t len = read(parent_io[0], buf, 1023); - - waitpid(pid, NULL, 0); - - if (len == 0) { - g_free(dmenu_input); - return; - } - } - - close(parent_io[0]); - - dispatch_menu_result(buf); + 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; + 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); + + return NULL; } /* vim: set tabstop=8 shiftwidth=8 expandtab textwidth=0: */ diff --git a/src/notification.c b/src/notification.c index b36a817..f4c6abf 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,35 @@ 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) +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) +{ + 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 +255,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 +284,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; @@ -274,6 +313,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 80d64ab..ffc887b 100644 --- a/src/notification.h +++ b/src/notification.h @@ -42,9 +42,13 @@ struct actions { gsize count; }; +typedef struct _notification_private NotificationPrivate; + struct notification { + NotificationPrivate *priv; int id; char *dbus_client; + bool dbus_valid; char *appname; char *summary; @@ -90,11 +94,23 @@ 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); +/** + * Retrieve the current reference count of the notification + */ +gint notification_refcount_get(struct notification *n); + +/** + * 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 +134,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/option_parser.c b/src/option_parser.c index 0cb8354..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,21 +200,6 @@ int ini_get_bool(const char *section, const char *key, int def) } } -char *clean_value(const char *value) -{ - char *s; - - if (value[0] == '"') - s = g_strdup(value + 1); - else - s = g_strdup(value); - - if (s[strlen(s) - 1] == '"') - s[strlen(s) - 1] = '\0'; - - return s; -} - int load_ini_file(FILE *fp) { if (!fp) 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/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; 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/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/notification.c b/test/notification.c index de189ad..b6afe2f 100644 --- a/test/notification.c +++ b/test/notification.c @@ -98,29 +98,52 @@ 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); 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_unref(a); + notification_unref(b); RUN_TEST(test_notification_replace_single_field); + RUN_TEST(test_notification_referencing); g_clear_pointer(&settings.icon_path, g_free); } 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); 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);