From 8579b3ed6bd3331e4cf6626e6f6423a166252117 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Tue, 18 Dec 2018 04:03:04 +0100 Subject: [PATCH] Add assertion macro to return values Just recently, I started using g_return_val_if_fail as a brief assertion checker. It'll also exit the function with a specified return value. But actually this introduces some weird behavior. It's configurable by environment variables and it'll print out a log message, if the expression didn't validate properly. But some of these assertions are actually ment to be silent. Using a simple macro makes it simple to structure the assertions and its return values in a block at the start of a function or anywhere else. --- src/dbus.c | 4 ++-- src/icon.c | 3 +-- src/log.c | 3 +-- src/markup.c | 21 +++++++-------------- src/menu.c | 6 ++---- src/notification.c | 9 +++------ src/option_parser.c | 22 +++++++--------------- src/queues.c | 6 ++---- src/settings.c | 3 +-- src/utils.c | 9 +++------ src/utils.h | 3 +++ src/x11/screen.c | 3 +-- src/x11/x.c | 9 +++------ 13 files changed, 36 insertions(+), 65 deletions(-) diff --git a/src/dbus.c b/src/dbus.c index 3994c4f..679a489 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -463,8 +463,8 @@ static bool dbus_get_fdn_daemon_info(GDBusConnection *connection, char **name, char **vendor) { - g_return_val_if_fail(pid, false); - g_return_val_if_fail(connection, false); + ASSERT_OR_RET(pid, false); + ASSERT_OR_RET(connection, false); char *owner = NULL; GError *error = NULL; diff --git a/src/icon.c b/src/icon.c index 1e9512c..93d4672 100644 --- a/src/icon.c +++ b/src/icon.c @@ -147,8 +147,7 @@ cairo_surface_t *icon_get_for_notification(const struct notification *n) else return NULL; - if (!pixbuf) - return NULL; + ASSERT_OR_RET(pixbuf, NULL); int w = gdk_pixbuf_get_width(pixbuf); int h = gdk_pixbuf_get_height(pixbuf); diff --git a/src/log.c b/src/log.c index a5e7de7..8e1ac54 100644 --- a/src/log.c +++ b/src/log.c @@ -30,8 +30,7 @@ static const char *log_level_to_string(GLogLevelFlags level) /* see log.h */ void log_set_level_from_string(const char *level) { - if (!level) - return; + ASSERT_OR_RET(level,); if (STR_CASEQ(level, "critical")) log_level = G_LOG_LEVEL_CRITICAL; diff --git a/src/markup.c b/src/markup.c index 3f74716..8877138 100644 --- a/src/markup.c +++ b/src/markup.c @@ -18,8 +18,7 @@ */ static char *markup_quote(char *str) { - if (!str) - return NULL; + ASSERT_OR_RET(str, NULL); str = string_replace_all("&", "&", str); str = string_replace_all("\"", """, str); @@ -36,8 +35,7 @@ static char *markup_quote(char *str) */ static char *markup_unquote(char *str) { - if (!str) - return NULL; + ASSERT_OR_RET(str, NULL); str = string_replace_all(""", "\"", str); str = string_replace_all("'", "'", str); @@ -54,8 +52,7 @@ static char *markup_unquote(char *str) */ static char *markup_br2nl(char *str) { - if (!str) - return NULL; + ASSERT_OR_RET(str, NULL); str = string_replace_all("
", "\n", str); str = string_replace_all("
", "\n", str); @@ -224,8 +221,7 @@ void markup_strip_img(char **str, char **urls) /* see markup.h */ char *markup_strip(char *str) { - if (!str) - return NULL; + ASSERT_OR_RET(str, NULL); /* strip all tags */ string_strip_delimited(str, '<', '>'); @@ -248,8 +244,7 @@ static bool markup_is_entity(const char *str) assert(*str == '&'); char *end = strchr(str, ';'); - if (!end) - return false; + ASSERT_OR_RET(end, false); // Parse (hexa)decimal entities with the format Ӓ or ઼ if (str[1] == '#') { @@ -293,8 +288,7 @@ static bool markup_is_entity(const char *str) */ static char *markup_escape_unsupported(char *str) { - if (!str) - return NULL; + ASSERT_OR_RET(str, NULL); char *match = str; while ((match = strchr(match, '&'))) { @@ -313,8 +307,7 @@ static char *markup_escape_unsupported(char *str) /* see markup.h */ char *markup_transform(char *str, enum markup_mode markup_mode) { - if (!str) - return NULL; + ASSERT_OR_RET(str, NULL); switch (markup_mode) { case MARKUP_NULL: diff --git a/src/menu.c b/src/menu.c index 1c52a5a..71f1a6b 100644 --- a/src/menu.c +++ b/src/menu.c @@ -222,8 +222,7 @@ void invoke_action(const char *action) */ void dispatch_menu_result(const char *input) { - if (!input) - return; + ASSERT_OR_RET(input,); char *in = g_strdup(input); g_strstrip(in); @@ -248,8 +247,7 @@ char *invoke_dmenu(const char *dmenu_input) return NULL; } - if (!dmenu_input || *dmenu_input == '\0') - return NULL; + ASSERT_OR_RET(STR_FULL(dmenu_input), NULL); gint dunst_to_dmenu; gint dmenu_to_dunst; diff --git a/src/notification.c b/src/notification.c index f9bde8a..968dd7b 100644 --- a/src/notification.c +++ b/src/notification.c @@ -168,8 +168,7 @@ int notification_cmp_data(const void *va, const void *vb, void *data) struct notification *a = (struct notification *) va; struct notification *b = (struct notification *) vb; - if (!settings.sort) - return 1; + ASSERT_OR_RET(settings.sort, 1); return notification_cmp(a, b); } @@ -191,8 +190,7 @@ int notification_is_duplicate(const struct notification *a, const struct notific /* see notification.h */ void rawimage_free(struct raw_image *i) { - if (!i) - return; + ASSERT_OR_RET(i,); g_free(i->data); g_free(i); @@ -220,8 +218,7 @@ void notification_ref(struct notification *n) /* see notification.h */ void notification_unref(struct notification *n) { - if (!n) - return; + ASSERT_OR_RET(n,); assert(n->priv->refcount > 0); if (!g_atomic_int_dec_and_test(&n->priv->refcount)) diff --git a/src/option_parser.c b/src/option_parser.c index 0bd78bb..85e780f 100644 --- a/src/option_parser.c +++ b/src/option_parser.c @@ -95,9 +95,7 @@ void add_entry(const char *section_name, const char *key, const char *value) const char *get_value(const char *section, const char *key) { struct section *s = get_section(section); - if (!s) { - return NULL; - } + ASSERT_OR_RET(s, NULL); for (int i = 0; i < s->entry_count; i++) { if (STR_EQ(s->entries[i].key, key)) { @@ -158,11 +156,8 @@ bool ini_is_set(const char *ini_section, const char *ini_key) const char *next_section(const char *section) { - if (section_count == 0) - return NULL; - - if (!section) - return sections[0].name; + ASSERT_OR_RET(section_count > 0, NULL); + ASSERT_OR_RET(section, sections[0].name); for (int i = 0; i < section_count; i++) { if (STR_EQ(section, sections[i].name)) { @@ -202,8 +197,7 @@ int ini_get_bool(const char *section, const char *key, int def) int load_ini_file(FILE *fp) { - if (!fp) - return 1; + ASSERT_OR_RET(fp, 1); char *line = NULL; size_t line_len = 0; @@ -277,9 +271,8 @@ void cmdline_load(int argc, char *argv[]) int cmdline_find_option(const char *key) { - if (!key) { - return -1; - } + ASSERT_OR_RET(key, -1); + char *key1 = g_strdup(key); char *key2 = strchr(key1, '/'); @@ -535,8 +528,7 @@ const char *cmdline_create_usage(void) /* see option_parser.h */ enum behavior_fullscreen parse_enum_fullscreen(const char *string, enum behavior_fullscreen def) { - if (!string) - return def; + ASSERT_OR_RET(string, def); if (STR_EQ(string, "show")) return FS_SHOW; diff --git a/src/queues.c b/src/queues.c index 2d47172..dcbb4e0 100644 --- a/src/queues.c +++ b/src/queues.c @@ -113,8 +113,7 @@ static void queues_swap_notifications(GQueue *queueA, */ static bool queues_notification_is_ready(const struct notification *n, struct dunst_status status, bool shown) { - if (!status.running) - return false; + ASSERT_OR_RET(status.running, false); if (status.fullscreen && shown) return n && n->fullscreen != FS_PUSHBACK; else if (status.fullscreen && !shown) @@ -418,8 +417,7 @@ void queues_update(struct dunst_status status) struct notification *n = iter->data; nextiter = iter->next; - if (!n) - return; + ASSERT_OR_RET(n,); if (!queues_notification_is_ready(n, status, false)) { iter = nextiter; diff --git a/src/settings.c b/src/settings.c index 64883d2..0e6104c 100644 --- a/src/settings.c +++ b/src/settings.c @@ -30,8 +30,7 @@ static const char *follow_mode_to_string(enum follow_mode f_mode) static enum follow_mode parse_follow_mode(const char *mode) { - if (!mode) - return FOLLOW_NONE; + ASSERT_OR_RET(mode, FOLLOW_NONE); if (STR_EQ(mode, "mouse")) return FOLLOW_MOUSE; diff --git a/src/utils.c b/src/utils.c index 4dcffa5..1b4713c 100644 --- a/src/utils.c +++ b/src/utils.c @@ -14,8 +14,7 @@ /* see utils.h */ char *string_replace_char(char needle, char replacement, char *haystack) { - if (!haystack) - return NULL; + ASSERT_OR_RET(haystack, NULL); char *current = haystack; while ((current = strchr(current, needle))) @@ -56,8 +55,7 @@ char *string_replace_at(char *buf, int pos, int len, const char *repl) /* see utils.h */ char *string_replace_all(const char *needle, const char *replacement, char *haystack) { - if (!haystack) - return NULL; + ASSERT_OR_RET(haystack, NULL); assert(needle); assert(replacement); @@ -104,8 +102,7 @@ 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; + ASSERT_OR_RET(value, NULL); size_t len = strlen(value); char *s; diff --git a/src/utils.h b/src/utils.h index 6b8a7a7..8a6f652 100644 --- a/src/utils.h +++ b/src/utils.h @@ -16,6 +16,9 @@ //! Test if string a and b are the same case-insensitively #define STR_CASEQ(a, b) (strcasecmp(a, b) == 0) +//! Assert that expr evaluates to true, if not return with val +#define ASSERT_OR_RET(expr, val) if (!(expr)) return val; + //! Convert a second into the internal time representation #define S2US(s) (((gint64)(s)) * 1000 * 1000) diff --git a/src/x11/screen.c b/src/x11/screen.c index f00431a..7ef1300 100644 --- a/src/x11/screen.c +++ b/src/x11/screen.c @@ -221,8 +221,7 @@ bool window_is_fullscreen(Window window) { bool fs = false; - if (!window) - return false; + ASSERT_OR_RET(window, false); Atom has_wm_state = XInternAtom(xctx.dpy, "_NET_WM_STATE", True); if (has_wm_state == None){ diff --git a/src/x11/x.c b/src/x11/x.c index 8a8a285..07d658f 100644 --- a/src/x11/x.c +++ b/src/x11/x.c @@ -674,8 +674,7 @@ void x_win_show(struct window_x11 *win) */ void x_win_hide(struct window_x11 *win) { - if (!win->visible) - return; + ASSERT_OR_RET(win->visible,); x_shortcut_ungrab(&settings.close_ks); x_shortcut_ungrab(&settings.close_all_ks); @@ -755,8 +754,7 @@ static int x_shortcut_tear_down_error_handler(void) */ static int x_shortcut_grab(struct keyboard_shortcut *ks) { - if (!ks->is_valid) - return 1; + ASSERT_OR_RET(ks->is_valid, 1); Window root; root = RootWindow(xctx.dpy, DefaultScreen(xctx.dpy)); @@ -805,8 +803,7 @@ static void x_shortcut_ungrab(struct keyboard_shortcut *ks) */ static void x_shortcut_init(struct keyboard_shortcut *ks) { - if (!ks|| !ks->str) - return; + ASSERT_OR_RET(ks && ks->str,); if (STR_EQ(ks->str, "none") || (STR_EQ(ks->str, ""))) { ks->is_valid = false;