From 59f994ee94942e6a8c7f879211d3a9f8ec4ba0c1 Mon Sep 17 00:00:00 2001 From: fwsmit <fw.smit01@gmail.com> Date: Mon, 25 Jan 2021 21:13:58 +0100 Subject: [PATCH 1/5] Extract icon path function from get_pixbuf_from_icon --- src/icon.c | 39 ++++++++++++++++++++++++++++++--------- src/icon.h | 11 +++++++++++ 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/icon.c b/src/icon.c index 3551eb5b5..fdca5595d 100644 --- a/src/icon.c +++ b/src/icon.c @@ -195,14 +195,14 @@ GdkPixbuf *get_pixbuf_from_file(const char *filename) return pixbuf; } -GdkPixbuf *get_pixbuf_from_icon(const char *iconname) +char *get_path_from_icon_name(const char *iconname) { if (STR_EMPTY(iconname)) return NULL; const char *suffixes[] = { ".svg", ".svgz", ".png", ".xpm", NULL }; - GdkPixbuf *pixbuf = NULL; gchar *uri_path = NULL; + char *new_name = NULL; if (g_str_has_prefix(iconname, "file://")) { uri_path = g_filename_from_uri(iconname, NULL, NULL); @@ -212,7 +212,7 @@ GdkPixbuf *get_pixbuf_from_icon(const char *iconname) /* absolute path? */ if (iconname[0] == '/' || iconname[0] == '~') { - pixbuf = get_pixbuf_from_file(iconname); + new_name = g_strdup(iconname); } else { /* search in icon_path */ char *start = settings.icon_path, @@ -224,26 +224,47 @@ GdkPixbuf *get_pixbuf_from_icon(const char *iconname) current_folder = g_strndup(start, end - start); for (const char **suf = suffixes; *suf; suf++) { - maybe_icon_path = g_strconcat(current_folder, "/", iconname, *suf, NULL); - if (is_readable_file(maybe_icon_path)) - pixbuf = get_pixbuf_from_file(maybe_icon_path); + gchar *name_with_extension = g_strconcat(iconname, *suf, NULL); + maybe_icon_path = g_build_filename(current_folder, name_with_extension, NULL); + if (is_readable_file(maybe_icon_path)) { + new_name = g_strdup(maybe_icon_path); + } + g_free(name_with_extension); g_free(maybe_icon_path); - if (pixbuf) + if (new_name) break; } g_free(current_folder); - if (pixbuf) + if (new_name) break; start = end + 1; } while (STR_FULL(end)); - if (!pixbuf) + if (!new_name) LOG_W("No icon found in path: '%s'", iconname); } g_free(uri_path); + return new_name; +} + +GdkPixbuf *get_pixbuf_from_icon(const char *iconname) +{ + char *path = get_path_from_icon_name(iconname); + if (!path) { + return NULL; + } + + GdkPixbuf *pixbuf = NULL; + + pixbuf = get_pixbuf_from_file(path); + g_free(path); + + if (!pixbuf) + LOG_W("No icon found in path: '%s'", iconname); + return pixbuf; } diff --git a/src/icon.h b/src/icon.h index 3134c78bb..242880b11 100644 --- a/src/icon.h +++ b/src/icon.h @@ -17,6 +17,17 @@ cairo_surface_t *gdk_pixbuf_to_cairo_surface(GdkPixbuf *pixbuf); */ GdkPixbuf *get_pixbuf_from_file(const char *filename); +/** Retrieve a path from an icon name. + * + * @param iconname A string describing a `file://` URL, an arbitary filename + * or an icon name, which then gets searched for in the + * settings.icon_path + * + * @return a newly allocated string with the icon path + * @retval NULL: file does not exist, not readable, etc.. + */ +char *get_path_from_icon_name(const char *iconname); + /** Retrieve an icon by its name sent via the notification bus, scaled according to settings * * @param iconname A string describing a `file://` URL, an arbitary filename From 293d71264e01567c161f69fe78f49aa4b165e1e7 Mon Sep 17 00:00:00 2001 From: fwsmit <fw.smit01@gmail.com> Date: Tue, 26 Jan 2021 14:20:09 +0100 Subject: [PATCH 2/5] Add tests for icon path I also fixed a test that was broken because of different behaviour. Now the first icon path is returned instead of the first valid icon path. --- test/icon.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 74 insertions(+), 2 deletions(-) diff --git a/test/icon.c b/test/icon.c index 94a29ce0a..b9ebc7b01 100644 --- a/test/icon.c +++ b/test/icon.c @@ -12,6 +12,67 @@ extern const char *base; +TEST test_get_path_from_icon_null(void){ + char *result = get_path_from_icon_name(NULL); + ASSERT_EQ(result, NULL); + PASS(); +} + +TEST test_get_path_from_icon_sorting(void) +{ + const char *iconpath = ICONPREFIX; + + const char* icon_names[] = { "onlypng", "onlysvg", "icon1" }; + const char* icon_paths[] = { "valid/onlypng.png", "valid/onlysvg.svg", "invalid/icon1.svg" }; + ASSERT_EQm("Test is incorrect", G_N_ELEMENTS(icon_names), G_N_ELEMENTS(icon_paths)); + + for (int i = 0; i < G_N_ELEMENTS(icon_names); i++){ + gchar *path = g_build_filename(base, iconpath, icon_paths[i], NULL); + char *result = get_path_from_icon_name(icon_names[i]); + ASSERT(result); + ASSERT_EQ(strcmp(result, path), 0); + g_free(path); + g_free(result); + } + + PASS(); +} + +TEST test_get_path_from_icon_name(void) +{ + const char *iconpath = ICONPREFIX; + + const char* icon_name = "onlypng"; + const char* expected_suffix = ".png"; + char* full_name = g_strconcat(icon_name, expected_suffix, NULL); + + gchar *path = g_build_filename(base, iconpath, "valid", full_name, NULL); + char *result = get_path_from_icon_name(icon_name); + + ASSERT(result); + ASSERT_EQ(strcmp(result, path), 0); + + g_free(full_name); + g_free(path); + g_free(result); + PASS(); +} + +TEST test_get_path_from_icon_name_full(void) +{ + const char *iconpath = ICONPREFIX; + + gchar *path = g_build_filename(base, iconpath, "valid", "icon1.svg", NULL); + + char *result = get_path_from_icon_name(path); + ASSERT(result); + ASSERT_EQ(strcmp(result, path), 0); + + g_free(path); + g_free(result); + PASS(); +} + TEST test_get_pixbuf_from_file_tilde(void) { const char *home = g_get_home_dir(); @@ -62,8 +123,8 @@ TEST test_get_pixbuf_from_icon_invalid(void) TEST test_get_pixbuf_from_icon_both(void) { GdkPixbuf *pixbuf = get_pixbuf_from_icon("icon1"); - ASSERT(pixbuf); - ASSERTm("SVG pixbuf hasn't precedence", IS_ICON_SVG(pixbuf)); + // the first icon found is invalid, so the pixbuf is empty + ASSERT(!pixbuf); g_clear_pointer(&pixbuf, g_object_unref); PASS(); @@ -170,12 +231,23 @@ TEST test_get_pixbuf_from_icon_both_is_scaled(void) SUITE(suite_icon) { + // set only valid icons in the path + settings.icon_path = g_strconcat( + base, ICONPREFIX "/valid" + ":", base, ICONPREFIX "/both", + NULL); + RUN_TEST(test_get_path_from_icon_name); + + g_clear_pointer(&settings.icon_path, g_free); settings.icon_path = g_strconcat( base, ICONPREFIX "/invalid" ":", base, ICONPREFIX "/valid" ":", base, ICONPREFIX "/both", NULL); + RUN_TEST(test_get_path_from_icon_null); + RUN_TEST(test_get_path_from_icon_sorting); + RUN_TEST(test_get_path_from_icon_name_full); RUN_TEST(test_get_pixbuf_from_file_tilde); RUN_TEST(test_get_pixbuf_from_file_absolute); RUN_TEST(test_get_pixbuf_from_icon_invalid); From 0d5cbfcfce24077e72f36934dcb70777548565e6 Mon Sep 17 00:00:00 2001 From: fwsmit <fw.smit01@gmail.com> Date: Thu, 7 Jan 2021 21:22:31 +0100 Subject: [PATCH 3/5] Set environment variables for scripts --- src/notification.c | 22 +++++++++++++++++++++- src/notification.h | 6 +++--- src/utils.c | 12 ++++++++++++ src/utils.h | 12 ++++++++++++ 4 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/notification.c b/src/notification.c index b2d70c71a..b91a3d408 100644 --- a/src/notification.c +++ b/src/notification.c @@ -133,6 +133,26 @@ void notification_run_script(struct notification *n) if (pid2) { exit(0); } else { + // Set environment variables + gchar *n_id_str = g_strdup_printf("%i", n->id); + gchar *n_progress_str = g_strdup_printf("%i", n->progress); + gchar *n_timeout_str = g_strdup_printf("%li", n->timeout/1000); + gchar *n_timestamp_str = g_strdup_printf("%li", n->timestamp / 1000); + char* icon_path = get_path_from_icon_name(icon); + safe_setenv("DUNST_APP_NAME", appname); + safe_setenv("DUNST_SUMMARY", summary); + safe_setenv("DUNST_BODY", body); + safe_setenv("DUNST_ICON_PATH", icon_path); + safe_setenv("DUNST_URGENCY", urgency); + safe_setenv("DUNST_ID", n_id_str); + safe_setenv("DUNST_PROGRESS", n_progress_str); + safe_setenv("DUNST_CATEGORY", n->category); + safe_setenv("DUNST_STACK_TAG", n->stack_tag); + safe_setenv("DUNST_URLS", n->urls); + safe_setenv("DUNST_TIMEOUT", n_timeout_str); + safe_setenv("DUNST_TIMESTAMP", n_timestamp_str); + safe_setenv("DUNST_STACK_TAG", n->stack_tag); + int ret = execlp(script, script, appname, @@ -142,7 +162,7 @@ void notification_run_script(struct notification *n) urgency, (char *)NULL); if (ret != 0) { - LOG_W("Unable to run script: %s", strerror(errno)); + LOG_W("Unable to run script %s: %s", n->scripts[i], strerror(errno)); exit(EXIT_FAILURE); } } diff --git a/src/notification.h b/src/notification.h index 93ca749fd..7457460b1 100644 --- a/src/notification.h +++ b/src/notification.h @@ -55,9 +55,9 @@ struct notification { char *iconname; /**< plain icon information (may be a path or just a name) Use this to compare the icon name with rules.*/ - gint64 start; /**< begin of current display */ - gint64 timestamp; /**< arrival time */ - gint64 timeout; /**< time to display */ + gint64 start; /**< begin of current display (in milliseconds) */ + gint64 timestamp; /**< arrival time (in milliseconds) */ + gint64 timeout; /**< time to display (in milliseconds) */ int locked; /**< If non-zero the notification is locked **/ GHashTable *actions; diff --git a/src/utils.c b/src/utils.c index cc2770f3d..f59c968cf 100644 --- a/src/utils.c +++ b/src/utils.c @@ -247,4 +247,16 @@ const char *user_get_home(void) return home_directory; } +bool safe_setenv(const char* key, const char* value){ + if (!key) + return false; + + if (!value) + setenv(key, "", 1); + else + setenv(key, value, 1); + + return true; +} + /* vim: set ft=c tabstop=8 shiftwidth=8 expandtab textwidth=0: */ diff --git a/src/utils.h b/src/utils.h index c0368bd4f..5e7ba6d03 100644 --- a/src/utils.h +++ b/src/utils.h @@ -4,6 +4,7 @@ #include <glib.h> #include <string.h> +#include <stdbool.h> //! Test if a string is NULL or empty #define STR_EMPTY(s) (!s || (*s == '\0')) @@ -137,5 +138,16 @@ gint64 time_monotonic_now(void); */ const char *user_get_home(void); +/** + * Try to set an environment variable safely. If an environment variable with + * name `key` exists, it will be overwritten. + * If `value` is null, `key` will be set to an empty string. + * + * @param key (nullable) The environment variable to change + * @param value (nullable) The value to change it to. + * @returns: A bool that is true when it succeeds + */ +bool safe_setenv(const char* key, const char* value); + #endif /* vim: set ft=c tabstop=8 shiftwidth=8 expandtab textwidth=0: */ From 52134ab8ceab9610552c92c0615cc8f2fcbab746 Mon Sep 17 00:00:00 2001 From: fwsmit <fw.smit01@gmail.com> Date: Thu, 21 Jan 2021 16:40:32 +0100 Subject: [PATCH 4/5] Add documentation for environment variables passed to scripts --- docs/dunst.5.pod | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/docs/dunst.5.pod b/docs/dunst.5.pod index 09a7bf310..aff803bb8 100644 --- a/docs/dunst.5.pod +++ b/docs/dunst.5.pod @@ -869,11 +869,22 @@ Within rules you can specify a script to be run every time the rule is matched by assigning the 'script' option to the name of the script to be run. When the script is called details of the notification that triggered it will be -passed via command line parameters in the following order: appname, summary, -body, icon, urgency. - -Where icon is the absolute path to the icon file if there is one and urgency is -one of "LOW", "NORMAL" or "CRITICAL". +passed via environment variables. The following variables are available: +B<DUNST_APP_NAME>, B<DUNST_SUMMARY>, B<DUNST_BODY>, B<DUNST_ICON_PATH>, +B<DUNST_URGENCY>, B<DUNST_ID>, B<DUNST_PROGRESS>, B<DUNST_CATEGORY>, +B<DUNST_STACK_TAG>, B<DUNST_URLS>, B<DUNST_TIMEOUT>, B<DUNST_TIMESTAMP> +and B<DUNST_STACK_TAG>. + +Another, less recommended way to get notifcations details from a script is via +command line parameters. These are passed to the script in the following order: +B<appname>, B<summary>, B<body>, B<icon_path>, B<urgency>. + +Where B<DUNST_ICON_PATH> or B<icon_path> is the absolute path to the icon file +if there is one. B<DUNST_URGENCY> or B<urgency> is one of "LOW", "NORMAL" or +"CRITICAL". B<DUNST_URLS> is a newline-separated list of urls associated with +the notification. + +Note that some variables may be empty. If the notification is suppressed, the script will not be run unless B<always_run_scripts> is set to true. From bc3de38aa71e67d6b369c0c2f26dd91357676c33 Mon Sep 17 00:00:00 2001 From: fwsmit <fw.smit01@gmail.com> Date: Thu, 21 Jan 2021 16:48:20 +0100 Subject: [PATCH 5/5] Slightly simplify and document script executing --- src/notification.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/notification.c b/src/notification.c index b91a3d408..7ed436495 100644 --- a/src/notification.c +++ b/src/notification.c @@ -129,6 +129,7 @@ void notification_run_script(struct notification *n) int status; waitpid(pid1, &status, 0); } else { + // second fork to prevent zombie processes int pid2 = fork(); if (pid2) { exit(0); @@ -153,7 +154,7 @@ void notification_run_script(struct notification *n) safe_setenv("DUNST_TIMESTAMP", n_timestamp_str); safe_setenv("DUNST_STACK_TAG", n->stack_tag); - int ret = execlp(script, + execlp(script, script, appname, summary, @@ -161,10 +162,9 @@ void notification_run_script(struct notification *n) icon, urgency, (char *)NULL); - if (ret != 0) { - LOG_W("Unable to run script %s: %s", n->scripts[i], strerror(errno)); - exit(EXIT_FAILURE); - } + + LOG_W("Unable to run script %s: %s", n->scripts[i], strerror(errno)); + exit(EXIT_FAILURE); } } }