From e4b0aae05a88b1e5bd2f986eec10f68326a0607c Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Fri, 6 Jul 2018 18:05:57 +0200 Subject: [PATCH 01/11] Replace free(val); val=NULL; with g_clear_pointer --- src/dbus.c | 6 ++---- src/option_parser.c | 3 +-- src/rules.c | 3 +-- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/dbus.c b/src/dbus.c index 2685137..20b6497 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -289,8 +289,7 @@ static notification *dbus_message_to_notification(const gchar *sender, GVariant n->transient = transient; if (actions->count < 1) { - actions_free(actions); - actions = NULL; + g_clear_pointer(&actions, actions_free); } n->actions = actions; @@ -484,8 +483,7 @@ static int dbus_get_fdn_daemon_info(GDBusConnection *connection, if (error) { /* Ignore the error, we may still be able to retrieve the PID */ - g_error_free(error); - error = NULL; + g_clear_pointer(&error, g_error_free); } else { g_variant_get(daemoninfo, "(ssss)", name, vendor, NULL, NULL); } diff --git a/src/option_parser.c b/src/option_parser.c index b95f911..465754b 100644 --- a/src/option_parser.c +++ b/src/option_parser.c @@ -66,9 +66,8 @@ void free_ini(void) g_free(sections[i].entries); g_free(sections[i].name); } - g_free(sections); + g_clear_pointer(§ions, g_free); section_count = 0; - sections = NULL; } section_t *get_section(const char *name) diff --git a/src/rules.c b/src/rules.c index 20d7d5a..92688bd 100644 --- a/src/rules.c +++ b/src/rules.c @@ -27,8 +27,7 @@ void rule_apply(rule_t *r, notification *n) if (r->new_icon) { g_free(n->icon); n->icon = g_strdup(r->new_icon); - rawimage_free(n->raw_icon); - n->raw_icon = NULL; + g_clear_pointer(&n->raw_icon, rawimage_free); } if (r->fg) { g_free(n->colors[ColFG]); From e091dd4d15375fc7749bd72889bdfe98be3643c3 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Fri, 6 Jul 2018 18:10:24 +0200 Subject: [PATCH 02/11] Notification comparison done right --- src/notification.c | 18 +++++++++--------- src/notification.h | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/notification.c b/src/notification.c index 1cea286..9e5dd9a 100644 --- a/src/notification.c +++ b/src/notification.c @@ -150,17 +150,11 @@ const char *notification_urgency_to_string(enum urgency urgency) } /* - * Helper function to compare to given + * Helper function to compare two given * notifications. */ -int notification_cmp(const void *va, const void *vb) +int notification_cmp(const notification *a, const notification *b) { - notification *a = (notification *) va; - notification *b = (notification *) vb; - - if (!settings.sort) - return 1; - if (a->urgency != b->urgency) { return b->urgency - a->urgency; } else { @@ -174,7 +168,13 @@ int notification_cmp(const void *va, const void *vb) */ int notification_cmp_data(const void *va, const void *vb, void *data) { - return notification_cmp(va, vb); + notification *a = (notification *) va; + notification *b = (notification *) vb; + + if (!settings.sort) + return 1; + + return notification_cmp(a, b); } int notification_is_duplicate(const notification *a, const notification *b) diff --git a/src/notification.h b/src/notification.h index a866214..fa1de64 100644 --- a/src/notification.h +++ b/src/notification.h @@ -89,8 +89,8 @@ void notification_init(notification *n); void actions_free(Actions *a); void rawimage_free(RawImage *i); void notification_free(notification *n); -int notification_cmp(const void *a, const void *b); -int notification_cmp_data(const void *a, const void *b, void *data); +int notification_cmp(const notification *a, const notification *b); +int notification_cmp_data(const void *va, const void *vb, void *data); int notification_is_duplicate(const notification *a, const notification *b); void notification_run_script(notification *n); void notification_print(notification *n); From f410f57211711ebecfc243867dda0895f5e6428f Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Fri, 6 Jul 2018 18:14:59 +0200 Subject: [PATCH 03/11] Use g_clear_pointer on fields, which may get reused --- src/dbus.c | 5 ++--- src/dunst.c | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/dbus.c b/src/dbus.c index 20b6497..718d5f5 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -124,7 +124,7 @@ static void on_get_capabilities(GDBusConnection *connection, g_variant_builder_add(builder, "s", "body-markup"); value = g_variant_new("(as)", builder); - g_variant_builder_unref(builder); + g_clear_pointer(&builder, g_variant_builder_unref); g_dbus_method_invocation_return_value(invocation, value); g_dbus_connection_flush(connection, NULL, NULL, NULL); @@ -615,8 +615,7 @@ int initdbus(void) void dbus_tear_down(int owner_id) { - if (introspection_data) - g_dbus_node_info_unref(introspection_data); + g_clear_pointer(&introspection_data, g_dbus_node_info_unref); g_bus_unown_name(owner_id); } diff --git a/src/dunst.c b/src/dunst.c index 20d7cf5..27c2872 100644 --- a/src/dunst.c +++ b/src/dunst.c @@ -176,7 +176,7 @@ int dunst_main(int argc, char *argv[]) run(NULL); g_main_loop_run(mainloop); - g_main_loop_unref(mainloop); + g_clear_pointer(&mainloop, g_main_loop_unref); /* remove signal handler watches */ g_source_remove(pause_src); From a229363dbdcb08899124fefcf3d9d24cda143096 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Fri, 6 Jul 2018 18:19:12 +0200 Subject: [PATCH 04/11] Simplify colored_layout codepaths --- src/draw.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/draw.c b/src/draw.c index 30bdb5e..17f24b9 100644 --- a/src/draw.c +++ b/src/draw.c @@ -625,10 +625,11 @@ void draw(void) bool first = true; for (GSList *iter = layouts; iter; iter = iter->next) { - if (iter->next) - dim = layout_render(image_surface, iter->data, iter->next->data, dim, first, iter->next == NULL); - else - dim = layout_render(image_surface, iter->data, NULL, dim, first, iter->next == NULL); + + colored_layout *cl_this = iter->data; + colored_layout *cl_next = iter->next ? iter->next->data : NULL; + + dim = layout_render(image_surface, cl_this, cl_next, dim, first, !cl_next); first = false; } From b5fbfd0e65926c0be4cc388dad825ac1f963bdaf Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Fri, 6 Jul 2018 18:35:50 +0200 Subject: [PATCH 05/11] Print progress in notification_print --- src/notification.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/notification.c b/src/notification.c index 9e5dd9a..df5b851 100644 --- a/src/notification.c +++ b/src/notification.c @@ -64,6 +64,7 @@ void notification_print(notification *n) printf("\tbg: %s\n", n->colors[ColBG]); printf("\tframe: %s\n", n->colors[ColFrame]); printf("\tfullscreen: %s\n", enum_to_string_fullscreen(n->fullscreen)); + printf("\tprogress: %d\n", n->progress); printf("\tid: %d\n", n->id); if (n->urls) { char *urls = string_replace_all("\n", "\t\t\n", g_strdup(n->urls)); From 718729053637fe7e844e57e52cd004ae91e40bb2 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Fri, 6 Jul 2018 18:40:25 +0200 Subject: [PATCH 06/11] Turn read only fields into const --- src/notification.c | 10 +++++----- src/notification.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/notification.c b/src/notification.c index df5b851..44730ee 100644 --- a/src/notification.c +++ b/src/notification.c @@ -98,10 +98,10 @@ void notification_run_script(notification *n) if (!n->script || strlen(n->script) < 1) return; - char *appname = n->appname ? n->appname : ""; - char *summary = n->summary ? n->summary : ""; - char *body = n->body ? n->body : ""; - char *icon = n->icon ? n->icon : ""; + const char *appname = n->appname ? n->appname : ""; + const char *summary = n->summary ? n->summary : ""; + const char *body = n->body ? n->body : ""; + const char *icon = n->icon ? n->icon : ""; const char *urgency = notification_urgency_to_string(n->urgency); @@ -134,7 +134,7 @@ void notification_run_script(notification *n) /* * Helper function to convert an urgency to a string */ -const char *notification_urgency_to_string(enum urgency urgency) +const char *notification_urgency_to_string(const enum urgency urgency) { switch (urgency) { case URG_NONE: diff --git a/src/notification.h b/src/notification.h index fa1de64..0b264ed 100644 --- a/src/notification.h +++ b/src/notification.h @@ -101,7 +101,7 @@ void notification_replace_single_field(char **haystack, void notification_update_text_to_render(notification *n); void notification_do_action(notification *n); -const char *notification_urgency_to_string(enum urgency urgency); +const char *notification_urgency_to_string(const enum urgency urgency); /** * Return the string representation for fullscreen behavior From f4fb95c827a01e32838b59d8230d91a100d8d884 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Sat, 7 Jul 2018 11:43:44 +0200 Subject: [PATCH 07/11] Do not make explicit NULL checks on elements Most of the NULL checks are actually doubly false logic. Turning the logic around and removing the NULL check makes the program easier to read. --- src/markup.c | 12 +++++------- src/menu.c | 2 +- src/notification.c | 8 ++++---- src/option_parser.c | 42 ++++++++++++++++++++---------------------- src/settings.c | 12 ++++++------ src/utils.c | 7 +++---- src/x11/screen.c | 2 +- src/x11/x.c | 2 +- test/option_parser.c | 2 +- 9 files changed, 42 insertions(+), 47 deletions(-) diff --git a/src/markup.c b/src/markup.c index 4ed6585..19d97ae 100644 --- a/src/markup.c +++ b/src/markup.c @@ -13,7 +13,7 @@ static char *markup_quote(char *str) { - assert(str != NULL); + assert(str); str = string_replace_all("&", "&", str); str = string_replace_all("\"", """, str); @@ -26,7 +26,7 @@ static char *markup_quote(char *str) static char *markup_unquote(char *str) { - assert(str != NULL); + assert(str); str = string_replace_all(""", "\"", str); str = string_replace_all("'", "'", str); @@ -39,7 +39,7 @@ static char *markup_unquote(char *str) static char *markup_br2nl(char *str) { - assert(str != NULL); + assert(str); str = string_replace_all("
", "\n", str); str = string_replace_all("
", "\n", str); @@ -230,9 +230,8 @@ void markup_strip_img(char **str, char **urls) */ char *markup_strip(char *str) { - if (str == NULL) { + if (!str) return NULL; - } /* strip all tags */ string_strip_delimited(str, '<', '>'); @@ -249,9 +248,8 @@ char *markup_strip(char *str) */ char *markup_transform(char *str, enum markup_mode markup_mode) { - if (str == NULL) { + if (!str) return NULL; - } switch (markup_mode) { case MARKUP_NULL: diff --git a/src/menu.c b/src/menu.c index ee90ed9..a32d55c 100644 --- a/src/menu.c +++ b/src/menu.c @@ -193,7 +193,7 @@ void dispatch_menu_result(const char *input) */ void context_menu(void) { - if (settings.dmenu_cmd == NULL) { + if (!settings.dmenu_cmd) { LOG_C("Unable to open dmenu: No dmenu command set."); return; } diff --git a/src/notification.c b/src/notification.c index 44730ee..7ec03bc 100644 --- a/src/notification.c +++ b/src/notification.c @@ -182,7 +182,7 @@ int notification_is_duplicate(const notification *a, const notification *b) { //Comparing raw icons is not supported, assume they are not identical if (settings.icon_position != icons_off - && (a->raw_icon != NULL || b->raw_icon != NULL)) + && (a->raw_icon || b->raw_icon)) return false; return strcmp(a->appname, b->appname) == 0 @@ -579,10 +579,10 @@ void notification_do_action(notification *n) context_menu(); } else if (n->urls) { - if (strstr(n->urls, "\n") == NULL) - open_browser(n->urls); - else + if (strstr(n->urls, "\n")) context_menu(); + else + open_browser(n->urls); } } diff --git a/src/option_parser.c b/src/option_parser.c index 465754b..df59030 100644 --- a/src/option_parser.c +++ b/src/option_parser.c @@ -83,9 +83,8 @@ section_t *get_section(const char *name) void add_entry(const char *section_name, const char *key, const char *value) { section_t *s = get_section(section_name); - if (s == NULL) { + if (!s) s = new_section(section_name); - } s->entry_count++; int len = s->entry_count; @@ -138,19 +137,19 @@ gint64 ini_get_time(const char *section, const char *key, gint64 def) int ini_get_int(const char *section, const char *key, int def) { const char *value = get_value(section, key); - if (value == NULL) - return def; - else + if (value) return atoi(value); + else + return def; } double ini_get_double(const char *section, const char *key, double def) { const char *value = get_value(section, key); - if (value == NULL) - return def; - else + if (value) return atof(value); + else + return def; } bool ini_is_set(const char *ini_section, const char *ini_key) @@ -163,9 +162,8 @@ const char *next_section(const char *section) if (section_count == 0) return NULL; - if (section == NULL) { + if (!section) return sections[0].name; - } for (int i = 0; i < section_count; i++) { if (strcmp(section, sections[i].name) == 0) { @@ -181,9 +179,7 @@ const char *next_section(const char *section) int ini_get_bool(const char *section, const char *key, int def) { const char *value = get_value(section, key); - if (value == NULL) - return def; - else { + if (value) { switch (value[0]) { case 'y': case 'Y': @@ -200,6 +196,8 @@ int ini_get_bool(const char *section, const char *key, int def) default: return def; } + } else { + return def; } } @@ -350,10 +348,10 @@ char *cmdline_get_string(const char *key, const char *def, const char *descripti if (str) return g_strdup(str); - if (def == NULL) - return NULL; - else + if (def) return g_strdup(def); + else + return NULL; } char *cmdline_get_path(const char *key, const char *def, const char *description) @@ -385,10 +383,10 @@ int cmdline_get_int(const char *key, int def, const char *description) cmdline_usage_append(key, "int", description); const char *str = cmdline_get_value(key); - if (str == NULL) - return def; - else + if (str) return atoi(str); + else + return def; } double cmdline_get_double(const char *key, double def, const char *description) @@ -396,10 +394,10 @@ double cmdline_get_double(const char *key, double def, const char *description) cmdline_usage_append(key, "double", description); const char *str = cmdline_get_value(key); - if (str == NULL) - return def; - else + if (str) return atof(str); + else + return def; } int cmdline_get_bool(const char *key, int def, const char *description) diff --git a/src/settings.c b/src/settings.c index 7f4e584..f278ac6 100644 --- a/src/settings.c +++ b/src/settings.c @@ -77,7 +77,7 @@ void load_settings(char *cmdline_config_path) xdgInitHandle(&xdg); - if (cmdline_config_path != NULL) { + if (cmdline_config_path) { if (0 == strcmp(cmdline_config_path, "-")) { config_file = stdin; } else { @@ -88,14 +88,14 @@ void load_settings(char *cmdline_config_path) DIE("Cannot find config file: '%s'", cmdline_config_path); } } - if (config_file == NULL) { + if (!config_file) { config_file = xdgConfigOpen("dunst/dunstrc", "r", &xdg); } - if (config_file == NULL) { + if (!config_file) { /* Fall back to just "dunstrc", which was used before 2013-06-23 * (before v0.2). */ config_file = xdgConfigOpen("dunstrc", "r", &xdg); - if (config_file == NULL) { + if (!config_file) { LOG_W("No dunstrc found."); xdgWipeHandle(&xdg); } @@ -669,7 +669,7 @@ void load_settings(char *cmdline_config_path) r = match; } - if (r == NULL) { + if (!r) { r = g_malloc(sizeof(rule_t)); rule_init(r); rules = g_slist_insert(rules, r, -1); @@ -689,7 +689,7 @@ void load_settings(char *cmdline_config_path) "markup", NULL ); - if (c != NULL) { + if (c) { r->markup = parse_markup_mode(c); g_free(c); } diff --git a/src/utils.c b/src/utils.c index 6eda3bc..8f06141 100644 --- a/src/utils.c +++ b/src/utils.c @@ -14,7 +14,7 @@ char *string_replace_char(char needle, char replacement, char *haystack) { char *current = haystack; - while ((current = strchr(current, needle)) != NULL) + while ((current = strchr(current, needle))) *current++ = replacement; return haystack; } @@ -49,9 +49,8 @@ char *string_replace(const char *needle, const char *replacement, char *haystack { char *start; start = strstr(haystack, needle); - if (start == NULL) { + if (!start) return haystack; - } return string_replace_at(haystack, (start - haystack), strlen(needle), replacement); } @@ -70,7 +69,7 @@ char *string_replace_all(const char *needle, const char *replacement, char *hays start = strstr(haystack, needle); repl_len = strlen(replacement); - while (start != NULL) { + while (start) { needle_pos = start - haystack; haystack = string_replace_at(haystack, needle_pos, needle_len, replacement); start = strstr(haystack + needle_pos + repl_len, needle); diff --git a/src/x11/screen.c b/src/x11/screen.c index 35df29c..1dfb4d2 100644 --- a/src/x11/screen.c +++ b/src/x11/screen.c @@ -46,7 +46,7 @@ static double get_xft_dpi_value(void) XrmInitialize(); char *xRMS = XResourceManagerString(xctx.dpy); - if (xRMS == NULL) { + if (!xRMS) { dpi = 0; return 0; } diff --git a/src/x11/x.c b/src/x11/x.c index 9311c10..60649d0 100644 --- a/src/x11/x.c +++ b/src/x11/x.c @@ -815,7 +815,7 @@ static void x_shortcut_ungrab(keyboard_shortcut *ks) */ static void x_shortcut_init(keyboard_shortcut *ks) { - if (ks == NULL || ks->str == NULL) + if (!ks|| !ks->str) return; if (!strcmp(ks->str, "none") || (!strcmp(ks->str, ""))) { diff --git a/test/option_parser.c b/test/option_parser.c index d5e283a..f2d0960 100644 --- a/test/option_parser.c +++ b/test/option_parser.c @@ -279,7 +279,7 @@ TEST test_option_get_bool(void) SUITE(suite_option_parser) { FILE *config_file = fopen("data/test-ini", "r"); - if (config_file == NULL) { + if (!config_file) { fputs("\nTest config file 'data/test-ini' couldn't be opened, failing.\n", stderr); exit(1); } From 54ce81ccca2f712d8d636e209fa4fe08b0aa5dea Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Sun, 8 Jul 2018 11:09:03 +0200 Subject: [PATCH 08/11] Convert notification.[ch] into docstrings --- src/notification.c | 65 ++++++++------------------------------------- src/notification.h | 66 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 54 deletions(-) diff --git a/src/notification.c b/src/notification.c index 7ec03bc..7291758 100644 --- a/src/notification.c +++ b/src/notification.c @@ -42,10 +42,7 @@ const char *enum_to_string_fullscreen(enum behavior_fullscreen in) } } -/* - * print a human readable representation - * of the given notification to stdout. - */ +/* see notification.h */ void notification_print(notification *n) { //TODO: use logging info for this @@ -89,10 +86,7 @@ void notification_print(notification *n) printf("}\n"); } -/* - * Run the script associated with the - * given notification. - */ +/* see notification.h */ void notification_run_script(notification *n) { if (!n->script || strlen(n->script) < 1) @@ -150,10 +144,7 @@ const char *notification_urgency_to_string(const enum urgency urgency) } } -/* - * Helper function to compare two given - * notifications. - */ +/* see notification.h */ int notification_cmp(const notification *a, const notification *b) { if (a->urgency != b->urgency) { @@ -163,10 +154,7 @@ int notification_cmp(const notification *a, const notification *b) } } -/* - * Wrapper for notification_cmp to match glib's - * compare functions signature. - */ +/* see notification.h */ int notification_cmp_data(const void *va, const void *vb, void *data) { notification *a = (notification *) va; @@ -192,10 +180,7 @@ int notification_is_duplicate(const notification *a, const notification *b) && a->urgency == b->urgency; } -/* - * Free the actions element - * @a: (nullable): Pointer to #Actions - */ +/* see notification.h */ void actions_free(Actions *a) { if (!a) @@ -206,10 +191,7 @@ void actions_free(Actions *a) g_free(a); } -/* - * Free a #RawImage - * @i: (nullable): pointer to #RawImage - */ +/* see notification.h */ void rawimage_free(RawImage *i) { if (!i) @@ -219,9 +201,7 @@ void rawimage_free(RawImage *i) g_free(i); } -/* - * Free the memory used by the given notification. - */ +/* see notification.h */ void notification_free(notification *n) { assert(n != NULL); @@ -244,14 +224,7 @@ void notification_free(notification *n) g_free(n); } -/* - * Replace the two chars where **needle points - * with a quoted "replacement", according to the markup settings. - * - * The needle is a double pointer and gets updated upon return - * to point to the first char, which occurs after replacement. - * - */ +/* see notification.h */ void notification_replace_single_field(char **haystack, char **needle, const char *replacement, @@ -275,14 +248,7 @@ void notification_replace_single_field(char **haystack, g_free(input); } -/* - * Create notification struct and initialise all fields with either - * - the default (if it's not needed to be freed later) - * - its undefined representation (NULL, -1) - * - * This function is guaranteed to return a valid pointer. - * @Returns: The generated notification - */ +/* see notification.h */ notification *notification_create(void) { notification *n = g_malloc0(sizeof(notification)); @@ -305,12 +271,7 @@ notification *notification_create(void) return n; } -/* - * Sanitize values of notification, apply all matching rules - * and generate derived fields. - * - * @n: the notification to sanitize - */ +/* see notification.h */ void notification_init(notification *n) { /* default to empty string to avoid further NULL faults */ @@ -558,11 +519,7 @@ void notification_update_text_to_render(notification *n) n->text_to_render = buf; } -/* - * If the notification has exactly one action, or one is marked as default, - * invoke it. If there are multiple and no default, open the context menu. If - * there are no actions, proceed similarly with urls. - */ +/* see notification.h */ void notification_do_action(notification *n) { if (n->actions) { diff --git a/src/notification.h b/src/notification.h index 0b264ed..47500f6 100644 --- a/src/notification.h +++ b/src/notification.h @@ -84,21 +84,87 @@ typedef struct _notification { char *urls; /**< urllist delimited by '\\n' */ } notification; +/** + * Create notification struct and initialise all fields with either + * - the default (if it's not needed to be freed later) + * - its undefined representation (NULL, -1) + * + * This function is guaranteed to return a valid pointer. + * @returns The generated notification + */ notification *notification_create(void); + +/** + * Sanitize values of notification, apply all matching rules + * and generate derived fields. + * + * @param n: the notification to sanitize + */ void notification_init(notification *n); + +/** + * Free the actions structure + * + * @param a (nullable): Pointer to #Actions + */ void actions_free(Actions *a); + +/** + * Free a #RawImage + * + * @param i (nullable): pointer to #RawImage + */ void rawimage_free(RawImage *i); + +/** + * Free the memory used by the given notification. + * + * @param n: pointer to #notification + */ void notification_free(notification *n); + +/** + * Helper function to compare two given notifications. + */ int notification_cmp(const notification *a, const notification *b); + +/** + * Wrapper for notification_cmp to match glib's + * compare functions signature. + */ int notification_cmp_data(const void *va, const void *vb, void *data); + int notification_is_duplicate(const notification *a, const notification *b); + +/** + * Run the script associated with the + * given notification. + */ void notification_run_script(notification *n); +/** + * print a human readable representation + * of the given notification to stdout. + */ void notification_print(notification *n); + +/** + * Replace the two chars where **needle points + * with a quoted "replacement", according to the markup settings. + * + * The needle is a double pointer and gets updated upon return + * to point to the first char, which occurs after replacement. + */ void notification_replace_single_field(char **haystack, char **needle, const char *replacement, enum markup_mode markup_mode); void notification_update_text_to_render(notification *n); + +/** + * If the notification has exactly one action, or one is marked as default, + * invoke it. If there are multiple and no default, open the context menu. If + * there are no actions, proceed similarly with urls. + */ void notification_do_action(notification *n); const char *notification_urgency_to_string(const enum urgency urgency); From 646d037b155d98b1bf58d1c7532343a5bed23646 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Sun, 8 Jul 2018 11:25:34 +0200 Subject: [PATCH 09/11] Make NULL a valid input for notification_free --- src/notification.c | 4 +++- src/notification.h | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/notification.c b/src/notification.c index 7291758..6d609cc 100644 --- a/src/notification.c +++ b/src/notification.c @@ -204,7 +204,9 @@ void rawimage_free(RawImage *i) /* see notification.h */ void notification_free(notification *n) { - assert(n != NULL); + if (!n) + return; + g_free(n->appname); g_free(n->summary); g_free(n->body); diff --git a/src/notification.h b/src/notification.h index 47500f6..ab21e3c 100644 --- a/src/notification.h +++ b/src/notification.h @@ -119,7 +119,7 @@ void rawimage_free(RawImage *i); /** * Free the memory used by the given notification. * - * @param n: pointer to #notification + * @param n (nullable): pointer to #notification */ void notification_free(notification *n); From 3c69e1f26370794af97f753b9c9660264595891d Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Sun, 8 Jul 2018 23:08:40 +0200 Subject: [PATCH 10/11] Remove doubly variables while converting dbus message --- src/dbus.c | 84 ++++++++++++++++-------------------------------------- 1 file changed, 25 insertions(+), 59 deletions(-) diff --git a/src/dbus.c b/src/dbus.c index 718d5f5..35fcc00 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -133,23 +133,10 @@ static void on_get_capabilities(GDBusConnection *connection, static notification *dbus_message_to_notification(const gchar *sender, GVariant *parameters) { - gchar *appname = NULL; - guint replaces_id = 0; - gchar *icon = NULL; - gchar *summary = NULL; - gchar *body = NULL; - Actions *actions = g_malloc0(sizeof(Actions)); - gint timeout = -1; + notification *n = notification_create(); - /* hints */ - gint urgency = 1; - gint progress = -1; - gboolean transient = 0; - gchar *fgcolor = NULL; - gchar *bgcolor = NULL; - gchar *frcolor = NULL; - gchar *category = NULL; - RawImage *raw_icon = NULL; + n->actions = g_malloc0(sizeof(Actions)); + n->dbus_client = g_strdup(sender); { GVariantIter *iter = g_variant_iter_new(parameters); @@ -161,65 +148,65 @@ static notification *dbus_message_to_notification(const gchar *sender, GVariant switch (idx) { case 0: if (g_variant_is_of_type(content, G_VARIANT_TYPE_STRING)) - appname = g_variant_dup_string(content, NULL); + n->appname = g_variant_dup_string(content, NULL); break; case 1: if (g_variant_is_of_type(content, G_VARIANT_TYPE_UINT32)) - replaces_id = g_variant_get_uint32(content); + n->id = g_variant_get_uint32(content); break; case 2: if (g_variant_is_of_type(content, G_VARIANT_TYPE_STRING)) - icon = g_variant_dup_string(content, NULL); + n->icon = g_variant_dup_string(content, NULL); break; case 3: if (g_variant_is_of_type(content, G_VARIANT_TYPE_STRING)) - summary = g_variant_dup_string(content, NULL); + n->summary = g_variant_dup_string(content, NULL); break; case 4: if (g_variant_is_of_type(content, G_VARIANT_TYPE_STRING)) - body = g_variant_dup_string(content, NULL); + n->body = g_variant_dup_string(content, NULL); break; case 5: if (g_variant_is_of_type(content, G_VARIANT_TYPE_STRING_ARRAY)) - actions->actions = g_variant_dup_strv(content, &(actions->count)); + n->actions->actions = g_variant_dup_strv(content, &(n->actions->count)); break; case 6: if (g_variant_is_of_type(content, G_VARIANT_TYPE_DICTIONARY)) { dict_value = g_variant_lookup_value(content, "urgency", G_VARIANT_TYPE_BYTE); if (dict_value) { - urgency = g_variant_get_byte(dict_value); + n->urgency = g_variant_get_byte(dict_value); g_variant_unref(dict_value); } dict_value = g_variant_lookup_value(content, "fgcolor", G_VARIANT_TYPE_STRING); if (dict_value) { - fgcolor = g_variant_dup_string(dict_value, NULL); + n->colors[ColFG] = g_variant_dup_string(dict_value, NULL); g_variant_unref(dict_value); } dict_value = g_variant_lookup_value(content, "bgcolor", G_VARIANT_TYPE_STRING); if (dict_value) { - bgcolor = g_variant_dup_string(dict_value, NULL); + n->colors[ColBG] = g_variant_dup_string(dict_value, NULL); g_variant_unref(dict_value); } dict_value = g_variant_lookup_value(content, "frcolor", G_VARIANT_TYPE_STRING); if (dict_value) { - frcolor = g_variant_dup_string(dict_value, NULL); + n->colors[ColFrame] = g_variant_dup_string(dict_value, NULL); g_variant_unref(dict_value); } dict_value = g_variant_lookup_value(content, "category", G_VARIANT_TYPE_STRING); if (dict_value) { - category = g_variant_dup_string(dict_value, NULL); + n->category = g_variant_dup_string(dict_value, NULL); g_variant_unref(dict_value); } dict_value = g_variant_lookup_value(content, "image-path", G_VARIANT_TYPE_STRING); if (dict_value) { - g_free(icon); - icon = g_variant_dup_string(dict_value, NULL); + g_free(n->icon); + n->icon = g_variant_dup_string(dict_value, NULL); g_variant_unref(dict_value); } @@ -229,7 +216,7 @@ static notification *dbus_message_to_notification(const gchar *sender, GVariant if (!dict_value) dict_value = g_variant_lookup_value(content, "icon_data", G_VARIANT_TYPE("(iiibiiay)")); if (dict_value) { - raw_icon = get_raw_image_from_data_hint(dict_value); + n->raw_icon = get_raw_image_from_data_hint(dict_value); g_variant_unref(dict_value); } @@ -240,28 +227,28 @@ static notification *dbus_message_to_notification(const gchar *sender, GVariant * So let's check for int and boolean until notify-send is fixed. */ if((dict_value = g_variant_lookup_value(content, "transient", G_VARIANT_TYPE_BOOLEAN))) { - transient = g_variant_get_boolean(dict_value); + n->transient = g_variant_get_boolean(dict_value); g_variant_unref(dict_value); } else if((dict_value = g_variant_lookup_value(content, "transient", G_VARIANT_TYPE_UINT32))) { - transient = g_variant_get_uint32(dict_value) > 0; + n->transient = g_variant_get_uint32(dict_value) > 0; g_variant_unref(dict_value); } else if((dict_value = g_variant_lookup_value(content, "transient", G_VARIANT_TYPE_INT32))) { - transient = g_variant_get_int32(dict_value) > 0; + n->transient = g_variant_get_int32(dict_value) > 0; g_variant_unref(dict_value); } if((dict_value = g_variant_lookup_value(content, "value", G_VARIANT_TYPE_INT32))) { - progress = g_variant_get_int32(dict_value); + n->progress = g_variant_get_int32(dict_value); g_variant_unref(dict_value); } else if((dict_value = g_variant_lookup_value(content, "value", G_VARIANT_TYPE_UINT32))) { - progress = g_variant_get_uint32(dict_value); + n->progress = g_variant_get_uint32(dict_value); g_variant_unref(dict_value); } } break; case 7: if (g_variant_is_of_type(content, G_VARIANT_TYPE_INT32)) - timeout = g_variant_get_int32(content); + n->timeout = g_variant_get_int32(content) * 1000; break; } g_variant_unref(content); @@ -273,29 +260,8 @@ static notification *dbus_message_to_notification(const gchar *sender, GVariant fflush(stdout); - notification *n = notification_create(); - - n->id = replaces_id; - n->appname = appname; - n->summary = summary; - n->body = body; - n->icon = icon; - n->raw_icon = raw_icon; - n->timeout = timeout < 0 ? -1 : timeout * 1000; - n->progress = progress; - n->urgency = urgency; - n->category = category; - n->dbus_client = g_strdup(sender); - n->transient = transient; - - if (actions->count < 1) { - g_clear_pointer(&actions, actions_free); - } - n->actions = actions; - - n->colors[ColFG] = fgcolor; - n->colors[ColBG] = bgcolor; - n->colors[ColFrame] = frcolor; + if (n->actions->count < 1) + g_clear_pointer(&n->actions, actions_free); notification_init(n); return n; From 368182f4d751c033d8a6e281e6a13ebeef753c44 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Mon, 9 Jul 2018 01:21:13 +0200 Subject: [PATCH 11/11] Log XEvents by their written names All XEvents only got logged with their IDs. It makes more sense to write out the XEvent's name. Also, writing that we received an XEvent and then logging again, that we ignored it, makes no sense. --- src/x11/screen.c | 9 ++++++--- src/x11/x.c | 10 +++++++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/x11/screen.c b/src/x11/screen.c index 1dfb4d2..185a607 100644 --- a/src/x11/screen.c +++ b/src/x11/screen.c @@ -144,10 +144,13 @@ static int autodetect_dpi(screen_info *scr) void screen_check_event(XEvent event) { - if (event.type == randr_event_base + RRScreenChangeNotify) + if (event.type == randr_event_base + RRScreenChangeNotify) { + LOG_D("XEvent: processing 'RRScreenChangeNotify'"); randr_update(); - else - LOG_D("XEvent: Ignored '%d'", event.type); + + } else { + LOG_D("XEvent: Ignoring '%d'", event.type); + } } void xinerama_update(void) diff --git a/src/x11/x.c b/src/x11/x.c index 60649d0..8e94598 100644 --- a/src/x11/x.c +++ b/src/x11/x.c @@ -274,21 +274,23 @@ gboolean x_mainloop_fd_dispatch(GSource *source, GSourceFunc callback, gpointer unsigned int state; while (XPending(xctx.dpy) > 0) { XNextEvent(xctx.dpy, &ev); - LOG_D("XEvent: processing '%d'", ev.type); switch (ev.type) { case Expose: + LOG_D("XEvent: processing 'Expose'"); if (ev.xexpose.count == 0 && win->visible) { draw(); } break; case ButtonRelease: + LOG_D("XEvent: processing 'ButtonRelease'"); if (ev.xbutton.window == win->xwin) { x_handle_click(ev); wake_up(); } break; case KeyPress: + LOG_D("XEvent: processing 'KeyPress'"); state = ev.xkey.state; /* NumLock is also encoded in the state. Remove it. */ state &= ~x_numlock_mod(); @@ -325,15 +327,21 @@ gboolean x_mainloop_fd_dispatch(GSource *source, GSourceFunc callback, gpointer } break; case FocusIn: + LOG_D("XEvent: processing 'FocusIn'"); + wake_up(); + break; case FocusOut: + LOG_D("XEvent: processing 'FocusOut'"); wake_up(); break; case CreateNotify: + LOG_D("XEvent: processing 'CreateNotify'"); if (win->visible && ev.xcreatewindow.override_redirect == 0) XRaiseWindow(xctx.dpy, win->xwin); break; case PropertyNotify: + LOG_D("XEvent: processing 'PropertyNotify'"); fullscreen_now = have_fullscreen_window(); scr = get_active_screen();