From 23ff4e753d68267096ea66e729337bb3653fc650 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 24 Aug 2018 14:52:49 +1000 Subject: [PATCH] ratbagd: wrap the various sd_bus functions into a macro Make sure we log an error if any of these fail, because most really never should, they're scaffolding we rely on to send the data. Some of these calls (the *_read() ones) could fail if a client sends wrong data, but we might as well log that. This patch only wraps calls that reply on the bus, normal sd_bus_* calls that we catch the error ourselves are handled as before. Likewise, any errors from libratbag are left as-is. We somewhat rely on libratbag to log something and such errors are not a failure of the actual scaffolding. This should make errors like https://github.com/libratbag/piper/issues/237 easier to debug. Signed-off-by: Peter Hutterer --- ratbagd/ratbagd-button.c | 52 ++++++++++------------ ratbagd/ratbagd-device.c | 49 ++++++++++----------- ratbagd/ratbagd-led.c | 47 ++++++++++---------- ratbagd/ratbagd-profile.c | 90 +++++++++++++++++---------------------- ratbagd/ratbagd.c | 32 ++++++-------- ratbagd/ratbagd.h | 10 +++++ 6 files changed, 134 insertions(+), 146 deletions(-) diff --git a/ratbagd/ratbagd-button.c b/ratbagd/ratbagd-button.c index af5e3139..444d583a 100644 --- a/ratbagd/ratbagd-button.c +++ b/ratbagd/ratbagd-button.c @@ -137,7 +137,9 @@ static int ratbagd_button_get_special(sd_bus *bus, verify_unsigned_int(special); - return sd_bus_message_append(reply, "u", special); + CHECK_CALL(sd_bus_message_append(reply, "u", special)); + + return 0; } static int ratbagd_button_set_special(sd_bus *bus, @@ -152,9 +154,7 @@ static int ratbagd_button_set_special(sd_bus *bus, enum ratbag_button_action_special special; int r; - r = sd_bus_message_read(m, "u", &special); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_read(m, "u", &special)); r = ratbag_button_set_special(button->lib_button, special); @@ -191,9 +191,7 @@ static int ratbagd_button_get_macro(sd_bus *bus, int r; unsigned int idx; - r = sd_bus_message_open_container(reply, 'a', "(uu)"); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_open_container(reply, 'a', "(uu)")); macro = ratbag_button_get_macro(button->lib_button); if (!macro) @@ -224,13 +222,13 @@ static int ratbagd_button_get_macro(sd_bus *bus, verify_unsigned_int(type); verify_unsigned_int(value); - r = sd_bus_message_append(reply, "(uu)", type, value); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_append(reply, "(uu)", type, value)); } out: - return sd_bus_message_close_container(reply); + CHECK_CALL(sd_bus_message_close_container(reply)); + + return 0; } static int ratbagd_button_set_macro(sd_bus *bus, @@ -246,9 +244,7 @@ static int ratbagd_button_set_macro(sd_bus *bus, int r, idx = 0; _cleanup_(ratbag_button_macro_unrefp) struct ratbag_button_macro *macro = NULL; - r = sd_bus_message_enter_container(m, 'a', "(uu)"); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_enter_container(m, 'a', "(uu)")); macro = ratbag_button_macro_new("macro"); while ((r = sd_bus_message_read(m, "(uu)", &type, &value)) > 0) { @@ -262,9 +258,7 @@ static int ratbagd_button_set_macro(sd_bus *bus, if (r < 0) return r; - r = sd_bus_message_exit_container(m); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_exit_container(m)); r = ratbag_button_set_macro(button->lib_button, macro); if (r < 0) { @@ -308,7 +302,9 @@ static int ratbagd_button_get_action_type(sd_bus *bus, verify_unsigned_int(type); - return sd_bus_message_append(reply, "u", type); + CHECK_CALL(sd_bus_message_append(reply, "u", type)); + + return 0; } static int ratbagd_button_get_action_types(sd_bus *bus, @@ -329,21 +325,19 @@ static int ratbagd_button_get_action_types(sd_bus *bus, enum ratbag_button_action_type *t; - r = sd_bus_message_open_container(reply, 'a', "u"); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_open_container(reply, 'a', "u")); ARRAY_FOR_EACH(types, t) { if (!ratbag_button_has_action_type(button->lib_button, *t)) continue; verify_unsigned_int(*t); - r = sd_bus_message_append(reply, "u", *t); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_append(reply, "u", *t)); } - return sd_bus_message_close_container(reply); + CHECK_CALL(sd_bus_message_close_container(reply)); + + return 0; } static int ratbagd_button_disable(sd_bus_message *m, @@ -353,9 +347,7 @@ static int ratbagd_button_disable(sd_bus_message *m, struct ratbagd_button *button = userdata; int r; - r = sd_bus_message_read(m, ""); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_read(m, "")); r = ratbag_button_disable(button->lib_button); if (r < 0) { @@ -365,7 +357,9 @@ static int ratbagd_button_disable(sd_bus_message *m, return r; } - return sd_bus_reply_method_return(m, "u", 0); + CHECK_CALL(sd_bus_reply_method_return(m, "u", 0)); + + return 0; } const sd_bus_vtable ratbagd_button_vtable[] = { diff --git a/ratbagd/ratbagd-device.c b/ratbagd/ratbagd-device.c index 181b0bf8..020be549 100644 --- a/ratbagd/ratbagd-device.c +++ b/ratbagd/ratbagd-device.c @@ -130,7 +130,9 @@ static int ratbagd_device_get_device_name(sd_bus *bus, name = ""; } - return sd_bus_message_append(reply, "s", name); + CHECK_CALL(sd_bus_message_append(reply, "s", name)); + + return 0; } static int ratbagd_device_get_svg(sd_bus *bus, @@ -151,7 +153,9 @@ static int ratbagd_device_get_svg(sd_bus *bus, svg = ""; } - return sd_bus_message_append(reply, "s", svg); + CHECK_CALL(sd_bus_message_append(reply, "s", svg)); + + return 0; } static int ratbagd_device_get_theme_svg(sd_bus_message *m, @@ -162,16 +166,13 @@ static int ratbagd_device_get_theme_svg(sd_bus_message *m, char svg_path[PATH_MAX] = {0}; const char *theme; const char *svg; - int r; const char *datadir; datadir = getenv("LIBRATBAG_DATA_DIR"); if (!datadir) datadir = LIBRATBAG_DATA_DIR; - r = sd_bus_message_read(m, "s", &theme); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_read(m, "s", &theme)); svg = ratbag_device_get_svg_name(device->lib_device); @@ -183,7 +184,9 @@ static int ratbagd_device_get_theme_svg(sd_bus_message *m, snprintf(svg_path, sizeof(svg_path), "%s/%s/%s", datadir, theme, svg); - return sd_bus_reply_method_return(m, "s", svg_path); + CHECK_CALL(sd_bus_reply_method_return(m, "s", svg_path)); + + return 0; } static int ratbagd_device_get_profiles(sd_bus *bus, @@ -199,23 +202,21 @@ static int ratbagd_device_get_profiles(sd_bus *bus, unsigned int i; int r; - r = sd_bus_message_open_container(reply, 'a', "o"); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_open_container(reply, 'a', "o")); for (i = 0; i < device->n_profiles; ++i) { profile = device->profiles[i]; if (!profile) continue; - r = sd_bus_message_append(reply, - "o", - ratbagd_profile_get_path(profile)); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_append(reply, + "o", + ratbagd_profile_get_path(profile))); } - return sd_bus_message_close_container(reply); + CHECK_CALL(sd_bus_message_close_container(reply)); + + return 0; } static int ratbagd_device_commit(sd_bus_message *m, @@ -233,7 +234,9 @@ static int ratbagd_device_commit(sd_bus_message *m, return r; } - return sd_bus_reply_method_return(m, "u", 0); + CHECK_CALL(sd_bus_reply_method_return(m, "u", 0)); + + return 0; } static int @@ -264,20 +267,18 @@ ratbagd_device_get_capabilities(sd_bus *bus, int r; size_t i; - r = sd_bus_message_open_container(reply, 'a', "u"); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_open_container(reply, 'a', "u")); for (i = 0; i < ELEMENTSOF(caps); i++) { cap = caps[i]; if (ratbag_device_has_capability(lib_device, cap)) { - r = sd_bus_message_append(reply, "u", cap); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_append(reply, "u", cap)); } } - return sd_bus_message_close_container(reply); + CHECK_CALL(sd_bus_message_close_container(reply)); + + return 0; } const sd_bus_vtable ratbagd_device_vtable[] = { diff --git a/ratbagd/ratbagd-led.c b/ratbagd/ratbagd-led.c index de8ea2ba..4101d7af 100644 --- a/ratbagd/ratbagd-led.c +++ b/ratbagd/ratbagd-led.c @@ -50,20 +50,19 @@ static int ratbagd_led_get_modes(sd_bus *bus, { struct ratbagd_led *led = userdata; enum ratbag_led_mode mode = 0; - int r; - r = sd_bus_message_open_container(reply, 'a', "u"); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_open_container(reply, 'a', "u")); while (mode <= RATBAG_LED_BREATHING) { if (ratbag_led_has_mode(led->lib_led, mode)) - sd_bus_message_append(reply, "u", mode); + CHECK_CALL(sd_bus_message_append(reply, "u", mode)); mode++; } - return sd_bus_message_close_container(reply); + CHECK_CALL(sd_bus_message_close_container(reply)); + + return 0; } static int ratbagd_led_get_mode(sd_bus *bus, @@ -81,7 +80,9 @@ static int ratbagd_led_get_mode(sd_bus *bus, verify_unsigned_int(mode); - return sd_bus_message_append(reply, "u", mode); + CHECK_CALL(sd_bus_message_append(reply, "u", mode)); + + return 0; } static int ratbagd_led_set_mode(sd_bus *bus, @@ -96,9 +97,7 @@ static int ratbagd_led_set_mode(sd_bus *bus, enum ratbag_led_mode mode; int r; - r = sd_bus_message_read(m, "u", &mode); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_read(m, "u", &mode)); r = ratbag_led_set_mode(led->lib_led, mode); @@ -129,7 +128,9 @@ static int ratbagd_led_get_type(sd_bus *bus, verify_unsigned_int(type); - return sd_bus_message_append(reply, "u", type); + CHECK_CALL(sd_bus_message_append(reply, "u", type)); + + return 0; } static int ratbagd_led_get_color(sd_bus *bus, @@ -144,7 +145,9 @@ static int ratbagd_led_get_color(sd_bus *bus, struct ratbag_color c; c = ratbag_led_get_color(led->lib_led); - return sd_bus_message_append(reply, "(uuu)", c.red, c.green, c.blue); + CHECK_CALL(sd_bus_message_append(reply, "(uuu)", c.red, c.green, c.blue)); + + return 0; } static int ratbagd_led_set_color(sd_bus *bus, @@ -159,9 +162,7 @@ static int ratbagd_led_set_color(sd_bus *bus, struct ratbag_color c; int r; - r = sd_bus_message_read(m, "(uuu)", &c.red, &c.green, &c.blue); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_read(m, "(uuu)", &c.red, &c.green, &c.blue)); if (c.red > 255) c.red = 255; @@ -199,7 +200,9 @@ static int ratbagd_led_get_effect_duration(sd_bus *bus, verify_unsigned_int(rate); - return sd_bus_message_append(reply, "u", rate); + CHECK_CALL(sd_bus_message_append(reply, "u", rate)); + + return 0; } static int ratbagd_led_set_effect_duration(sd_bus *bus, @@ -214,9 +217,7 @@ static int ratbagd_led_set_effect_duration(sd_bus *bus, unsigned int rate; int r; - r = sd_bus_message_read(m, "u", &rate); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_read(m, "u", &rate)); if (rate > 10000) rate = 10000; @@ -250,7 +251,9 @@ static int ratbagd_led_get_brightness(sd_bus *bus, verify_unsigned_int(brightness); - return sd_bus_message_append(reply, "u", brightness); + CHECK_CALL(sd_bus_message_append(reply, "u", brightness)); + + return 0; } static int ratbagd_led_set_brightness(sd_bus *bus, @@ -265,9 +268,7 @@ static int ratbagd_led_set_brightness(sd_bus *bus, unsigned int brightness; int r; - r = sd_bus_message_read(m, "u", &brightness); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_read(m, "u", &brightness)); if (brightness > 255) brightness = 255; diff --git a/ratbagd/ratbagd-profile.c b/ratbagd/ratbagd-profile.c index 77f9e7ed..c6b43300 100644 --- a/ratbagd/ratbagd-profile.c +++ b/ratbagd/ratbagd-profile.c @@ -102,25 +102,22 @@ static int ratbagd_profile_get_resolutions(sd_bus *bus, struct ratbagd_profile *profile = userdata; struct ratbagd_resolution *resolution; unsigned int i; - int r; - r = sd_bus_message_open_container(reply, 'a', "o"); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_open_container(reply, 'a', "o")); for (i = 0; i < profile->n_resolutions; ++i) { resolution = profile->resolutions[i]; if (!resolution) continue; - r = sd_bus_message_append(reply, - "o", - ratbagd_resolution_get_path(resolution)); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_append(reply, + "o", + ratbagd_resolution_get_path(resolution))); } - return sd_bus_message_close_container(reply); + CHECK_CALL(sd_bus_message_close_container(reply)); + + return 0; } static int ratbagd_profile_get_buttons(sd_bus *bus, @@ -134,25 +131,22 @@ static int ratbagd_profile_get_buttons(sd_bus *bus, struct ratbagd_profile *profile = userdata; struct ratbagd_button *button; unsigned int i; - int r; - r = sd_bus_message_open_container(reply, 'a', "o"); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_open_container(reply, 'a', "o")); for (i = 0; i < profile->n_buttons; ++i) { button = profile->buttons[i]; if (!button) continue; - r = sd_bus_message_append(reply, + CHECK_CALL(sd_bus_message_append(reply, "o", - ratbagd_button_get_path(button)); - if (r < 0) - return r; + ratbagd_button_get_path(button))); } - return sd_bus_message_close_container(reply); + CHECK_CALL(sd_bus_message_close_container(reply)); + + return 0; } static int ratbagd_profile_get_leds(sd_bus *bus, @@ -166,25 +160,22 @@ static int ratbagd_profile_get_leds(sd_bus *bus, struct ratbagd_profile *profile = userdata; struct ratbagd_led *led; unsigned int i; - int r; - r = sd_bus_message_open_container(reply, 'a', "o"); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_open_container(reply, 'a', "o")); for (i = 0; i < profile->n_leds; ++i) { led = profile->leds[i]; if (!led) continue; - r = sd_bus_message_append(reply, - "o", - ratbagd_led_get_path(led)); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_append(reply, + "o", + ratbagd_led_get_path(led))); } - return sd_bus_message_close_container(reply); + CHECK_CALL(sd_bus_message_close_container(reply)); + + return 0; } static int ratbagd_profile_is_active(sd_bus *bus, @@ -200,7 +191,9 @@ static int ratbagd_profile_is_active(sd_bus *bus, is_active = ratbag_profile_is_active(profile->lib_profile); - return sd_bus_message_append(reply, "b", is_active); + CHECK_CALL(sd_bus_message_append(reply, "b", is_active)); + + return 0; } static int ratbagd_profile_find_button(sd_bus *bus, @@ -286,9 +279,7 @@ static int ratbagd_profile_set_active(sd_bus_message *m, struct ratbagd_profile *profile = userdata; int r; - r = sd_bus_message_read(m, ""); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_read(m, "")); r = ratbag_profile_set_active(profile->lib_profile); if (r < 0) { @@ -302,7 +293,9 @@ static int ratbagd_profile_set_active(sd_bus_message *m, profile->device, ratbagd_profile_active_signal_cb); - return sd_bus_reply_method_return(m, "u", 0); + CHECK_CALL(sd_bus_reply_method_return(m, "u", 0)); + + return 0; } static int @@ -318,9 +311,7 @@ ratbagd_profile_set_enabled(sd_bus *bus, int enabled; int r; - r = sd_bus_message_read(m, "b", &enabled); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_read(m, "b", &enabled)); r = ratbag_profile_set_enabled(profile->lib_profile, enabled); if (r == 0) { @@ -347,7 +338,9 @@ ratbagd_profile_is_enabled(sd_bus *bus, struct ratbagd_profile *profile = userdata; int enabled = ratbag_profile_is_enabled(profile->lib_profile) != 0; - return sd_bus_message_append(reply, "b", enabled); + CHECK_CALL(sd_bus_message_append(reply, "b", enabled)); + + return 0; } static int @@ -363,9 +356,7 @@ ratbagd_profile_set_name(sd_bus *bus, char *name; int r; - r = sd_bus_message_read(m, "s", &name); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_read(m, "s", &name)); r = ratbag_profile_set_name(profile->lib_profile, name); @@ -392,7 +383,9 @@ ratbagd_profile_get_name(sd_bus *bus, struct ratbagd_profile *profile = userdata; const char *name = ratbag_profile_get_name(profile->lib_profile); - return sd_bus_message_append(reply, "s", name); + CHECK_CALL(sd_bus_message_append(reply, "s", name)); + + return 0; } static int @@ -410,23 +403,20 @@ ratbagd_profile_get_capabilities(sd_bus *bus, enum ratbag_profile_capability caps[] = { RATBAG_PROFILE_CAP_WRITABLE_NAME, }; - int r; size_t i; - r = sd_bus_message_open_container(reply, 'a', "u"); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_open_container(reply, 'a', "u")); for (i = 0; i < ELEMENTSOF(caps); i++) { cap = caps[i]; if (ratbag_profile_has_capability(lib_profile, cap)) { - r = sd_bus_message_append(reply, "u", cap); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_append(reply, "u", cap)); } } - return sd_bus_message_close_container(reply); + CHECK_CALL(sd_bus_message_close_container(reply)); + + return 0; } const sd_bus_vtable ratbagd_profile_vtable[] = { diff --git a/ratbagd/ratbagd.c b/ratbagd/ratbagd.c index 1aa2b9db..ee2bcaee 100644 --- a/ratbagd/ratbagd.c +++ b/ratbagd/ratbagd.c @@ -142,21 +142,18 @@ static int ratbagd_get_devices(sd_bus *bus, { struct ratbagd *ctx = userdata; struct ratbagd_device *device; - int r; - r = sd_bus_message_open_container(reply, 'a', "o"); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_open_container(reply, 'a', "o")); RATBAGD_DEVICE_FOREACH(device, ctx) { - r = sd_bus_message_append(reply, - "o", - ratbagd_device_get_path(device)); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_append(reply, + "o", + ratbagd_device_get_path(device))); } - return sd_bus_message_close_container(reply); + CHECK_CALL(sd_bus_message_close_container(reply)); + + return 0; } static int ratbagd_get_themes(sd_bus *bus, @@ -169,23 +166,18 @@ static int ratbagd_get_themes(sd_bus *bus, { struct ratbagd *ctx = userdata; const char **theme; - int r; - r = sd_bus_message_open_container(reply, 'a', "s"); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_open_container(reply, 'a', "s")); theme = ctx->themes; while(*theme) { - r = sd_bus_message_append(reply, - "s", - *theme); - if (r < 0) - return r; + CHECK_CALL(sd_bus_message_append(reply, "s", *theme)); theme++; } - return sd_bus_message_close_container(reply); + CHECK_CALL(sd_bus_message_close_container(reply)); + + return 0; } static const sd_bus_vtable ratbagd_vtable[] = { diff --git a/ratbagd/ratbagd.h b/ratbagd/ratbagd.h index 28b7d643..2c534b75 100644 --- a/ratbagd/ratbagd.h +++ b/ratbagd/ratbagd.h @@ -57,6 +57,16 @@ struct ratbagd_led; void log_verbose(const char *fmt, ...) _printf_(1, 2); void log_error(const char *fmt, ...) _printf_(1, 2); +#define CHECK_CALL(_call) \ + do { \ + int _r = _call; \ + if (_r < 0) { \ + log_error("%s: '%s' failed with: %s\n", __func__, #_call, strerror(-_r)); \ + return _r; \ + } \ + } while (0) + + /* * Profiles */