From 1abc7cdb692f135e2a794cdaff51be7a4b2e8c3f Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 12 Oct 2015 08:51:33 +1000 Subject: [PATCH 1/3] Add strdup_safe() as a reliable strdup function Handles NULL and guarantees a successful strdup Fixes #14 Signed-off-by: Peter Hutterer --- src/libratbag-hidraw.c | 2 +- src/libratbag-util.h | 18 ++++++++++++++++++ src/libratbag.c | 4 ++-- tools/ratbag-command.c | 2 +- tools/shared.c | 12 ++++++------ 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/libratbag-hidraw.c b/src/libratbag-hidraw.c index d712806e..2a29bef6 100644 --- a/src/libratbag-hidraw.c +++ b/src/libratbag-hidraw.c @@ -186,7 +186,7 @@ ratbag_open_hidraw_node(struct ratbag_device *device, struct udev_device *hidraw device->hidraw.reports = zalloc(reports_size); ratbag_hidraw_parse_report_descriptor(device); - device->hidraw.sysname = strdup(sysname); + device->hidraw.sysname = strdup_safe(sysname); return 0; err: diff --git a/src/libratbag-util.h b/src/libratbag-util.h index 3bd12a2a..bb79c028 100644 --- a/src/libratbag-util.h +++ b/src/libratbag-util.h @@ -99,6 +99,24 @@ zalloc(size_t size) return p; } +/** + * returns NULL if str is NULL, otherwise guarantees a successful strdup. + */ +static inline char * +strdup_safe(const char *str) +{ + char *s; + + if (!str) + return NULL; + + s = strdup(str); + if (!s) + abort(); + + return s; +} + static inline void msleep(unsigned int ms) { diff --git a/src/libratbag.c b/src/libratbag.c index 4ad3aa99..5235672f 100644 --- a/src/libratbag.c +++ b/src/libratbag.c @@ -157,7 +157,7 @@ ratbag_device_new(struct ratbag *ratbag, struct udev_device *udev_device, struct ratbag_device *device = NULL; device = zalloc(sizeof(*device)); - device->name = strdup(name); + device->name = strdup_safe(name); if (!name) { free(device); @@ -1190,7 +1190,7 @@ ratbag_button_set_macro(struct ratbag_button *button, const char *name) } button->action.type = RATBAG_BUTTON_ACTION_TYPE_MACRO; - button->action.macro->name = name ? strdup(name) : NULL; + button->action.macro->name = strdup_safe(name); return 0; } diff --git a/tools/ratbag-command.c b/tools/ratbag-command.c index a332438b..5ba24adc 100644 --- a/tools/ratbag-command.c +++ b/tools/ratbag-command.c @@ -481,7 +481,7 @@ str_to_macro(const char *action_arg, struct macro *m) if (!action_arg) return -EINVAL; - str = strdup(action_arg); + str = strdup_safe(action_arg); s = str; m->name = ""; diff --git a/tools/shared.c b/tools/shared.c index 29202bab..1e7fa19a 100644 --- a/tools/shared.c +++ b/tools/shared.c @@ -131,7 +131,7 @@ button_action_button_to_str(struct ratbag_button *button) sprintf(str, "button %d", ratbag_button_get_button(button)); - return strdup(str); + return strdup_safe(str); } char * @@ -145,7 +145,7 @@ button_action_key_to_str(struct ratbag_button *button) if (!str) str = "UNKNOWN"; - return strdup(str); + return strdup_safe(str); } static const char *strip_ev_key(int key) @@ -191,7 +191,7 @@ button_action_macro_to_str(struct ratbag_button *button) } } - return strdup(str); + return strdup_safe(str); } char * @@ -205,12 +205,12 @@ button_action_to_str(struct ratbag_button *button) switch (type) { case RATBAG_BUTTON_ACTION_TYPE_BUTTON: str = button_action_button_to_str(button); break; case RATBAG_BUTTON_ACTION_TYPE_KEY: str = button_action_key_to_str(button); break; - case RATBAG_BUTTON_ACTION_TYPE_SPECIAL: str = strdup(button_action_special_to_str(button)); break; + case RATBAG_BUTTON_ACTION_TYPE_SPECIAL: str = strdup_safe(button_action_special_to_str(button)); break; case RATBAG_BUTTON_ACTION_TYPE_MACRO: str = button_action_macro_to_str(button); break; - case RATBAG_BUTTON_ACTION_TYPE_NONE: str = strdup("none"); break; + case RATBAG_BUTTON_ACTION_TYPE_NONE: str = strdup_safe("none"); break; default: error("type %d unknown\n", type); - str = strdup("UNKNOWN"); + str = strdup_safe("UNKNOWN"); } return str; From 5b8f3c4744d75332312ecd9f000e3300861ca529 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 12 Oct 2015 09:14:24 +1000 Subject: [PATCH 2/3] Add snprintf_safe() and sprintf_safe helpers Blows up when sprintf encounters an error, the result string is empty or the result string is truncated. Note: sprintf_safe only works on static buffers, it uses ARRAY_LENGTH. Fixes #14 Signed-off-by: Peter Hutterer --- src/hidpp20.c | 2 +- src/libratbag-util.h | 20 ++++++++++++++++++++ src/libratbag.c | 4 ++-- tools/ratbag-command.c | 2 +- tools/shared.c | 2 +- 5 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/hidpp20.c b/src/hidpp20.c index d5c555cb..77d8909f 100644 --- a/src/hidpp20.c +++ b/src/hidpp20.c @@ -63,7 +63,7 @@ hidpp20_feature_get_name(uint16_t feature) CASE_RETURN_STRING(HIDPP_PAGE_ONBOARD_PROFILES); CASE_RETURN_STRING(HIDPP_PAGE_MOUSE_BUTTON_SPY); default: - sprintf(numeric, "%#4x", feature); + sprintf_safe(numeric, "%#4x", feature); str = numeric; break; } diff --git a/src/libratbag-util.h b/src/libratbag-util.h index bb79c028..bd8d70e6 100644 --- a/src/libratbag-util.h +++ b/src/libratbag-util.h @@ -25,6 +25,7 @@ #define LIBRATBAG_UTIL_H #include +#include #include #include #include @@ -117,6 +118,25 @@ strdup_safe(const char *str) return s; } +static inline int +snprintf_safe(char *buf, size_t n, const char *fmt, ...) +{ + va_list args; + int rc; + + va_start(args, fmt); + rc = vsnprintf(buf, n, fmt, args); + va_end(args); + + if (rc < 0 || n < (size_t)rc) + abort(); + + return rc; +} + +#define sprintf_safe(buf, fmt, ...) \ + snprintf_safe(buf, ARRAY_LENGTH(buf), fmt, __VA_ARGS__) + static inline void msleep(unsigned int ms) { diff --git a/src/libratbag.c b/src/libratbag.c index 5235672f..ae676efd 100644 --- a/src/libratbag.c +++ b/src/libratbag.c @@ -106,10 +106,10 @@ log_buffer(struct ratbag *ratbag, output_buf = zalloc(buf_len); n = 0; if (header) - n += sprintf(output_buf, "%s", header); + n += snprintf_safe(output_buf, buf_len - n, "%s", header); for (i = 0; i < len; ++i) { - n += sprintf(&output_buf[n], "%s%02x", sep, buf[i] & 0xFF); + n += snprintf_safe(&output_buf[n], buf_len - n, "%s%02x", sep, buf[i] & 0xFF); sep = " "; } diff --git a/tools/ratbag-command.c b/tools/ratbag-command.c index 5ba24adc..62ace523 100644 --- a/tools/ratbag-command.c +++ b/tools/ratbag-command.c @@ -706,7 +706,7 @@ ratbag_cmd_list_supported_devices(const struct ratbag_cmd *cmd, i = -1; while (++i < n) { - sprintf(path, "/dev/input/%s", input_list[i]->d_name); + sprintf_safe(path, "/dev/input/%s", input_list[i]->d_name); device = ratbag_cmd_open_device(ratbag, path); if (device) { printf("%s:\t%s\n", path, ratbag_device_get_name(device)); diff --git a/tools/shared.c b/tools/shared.c index 1e7fa19a..e9c137fb 100644 --- a/tools/shared.c +++ b/tools/shared.c @@ -129,7 +129,7 @@ button_action_button_to_str(struct ratbag_button *button) { char str[96]; - sprintf(str, "button %d", ratbag_button_get_button(button)); + sprintf_safe(str, "button %d", ratbag_button_get_button(button)); return strdup_safe(str); } From 46b7a156636263d050ea697a30a59c82abdbd854 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 12 Oct 2015 09:43:40 +1000 Subject: [PATCH 3/3] Start using the __attribute__((cleanup)) Not a lot of use-cases in the main libratbag code yet, but the protocol handling and hidraw bits may make more use of it. --- src/libratbag-private.h | 22 ++++++++++++++++++++++ src/libratbag-util.h | 8 ++++++++ src/libratbag.c | 11 +++-------- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/libratbag-private.h b/src/libratbag-private.h index d5dfd878..7e251cdc 100644 --- a/src/libratbag-private.h +++ b/src/libratbag-private.h @@ -33,6 +33,28 @@ #include "libratbag-util.h" #include "libratbag-hidraw.h" +static inline void +cleanup_device(struct ratbag_device **d) +{ + ratbag_device_unref(*d); +} + +static inline void +cleanup_profile(struct ratbag_profile **p) +{ + ratbag_profile_unref(*p); +} + +static inline void +cleanup_resolution(struct ratbag_resolution **r) +{ + ratbag_resolution_unref(*r); +} + +#define _cleanup_device_ _cleanup_(cleanup_device) +#define _cleanup_profile_ _cleanup_(cleanup_profile) +#define _cleanup_resolution_ _cleanup_(cleanup_resolution) + #define BUS_ANY 0xffff #define VENDOR_ANY 0xffff #define PRODUCT_ANY 0xffff diff --git a/src/libratbag-util.h b/src/libratbag-util.h index bd8d70e6..cbe4a329 100644 --- a/src/libratbag-util.h +++ b/src/libratbag-util.h @@ -80,6 +80,14 @@ int list_empty(const struct list *list); #define streq(s1, s2) (strcmp((s1), (s2)) == 0) #define strneq(s1, s2, n) (strncmp((s1), (s2), (n)) == 0) +static inline void +cleanup_free(void *p) { + free(*(void**)p); +} + +#define _cleanup_(x) __attribute__((cleanup(x))) +#define _cleanup_free_ _cleanup_(cleanup_free) + static inline char* strncpy_safe(char *dest, const char *src, size_t n) { diff --git a/src/libratbag.c b/src/libratbag.c index ae676efd..ebe9dd36 100644 --- a/src/libratbag.c +++ b/src/libratbag.c @@ -90,7 +90,7 @@ log_buffer(struct ratbag *ratbag, const char *header, uint8_t *buf, size_t len) { - char *output_buf; + _cleanup_free_ char *output_buf = NULL; char *sep = ""; unsigned int i, n; unsigned int buf_len; @@ -114,8 +114,6 @@ log_buffer(struct ratbag *ratbag, } log_msg(ratbag, priority, "%s\n", output_buf); - - free(output_buf); } LIBRATBAG_EXPORT void @@ -208,8 +206,8 @@ static inline bool ratbag_sanity_check_device(struct ratbag_device *device) { struct ratbag *ratbag = device->ratbag; - struct ratbag_profile *profile = NULL; - struct ratbag_resolution *res = NULL; + _cleanup_profile_ struct ratbag_profile *profile = NULL; + _cleanup_resolution_ struct ratbag_resolution *res = NULL; bool has_active = false; bool has_default = false; unsigned int nres, nprofiles; @@ -278,9 +276,6 @@ ratbag_sanity_check_device(struct ratbag_device *device) rc = true; out: - ratbag_profile_unref(profile); - ratbag_resolution_unref(res); - return rc; }