From ca8c3e73372babbf1aaa47db55a9360739ca37c5 Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Tue, 12 Jun 2018 00:52:29 +0200 Subject: [PATCH 1/7] No longer use a temporary buffer in the die() function Before the following change f947d0a Breaks configfiles! Major refactoring of i3status, see below The die(fmt, ...) function was outputting the reason to the status bar in addition to stderr. For this reason, it was meaningful to create a temporary string according to the format string and then passing it around to the different functions. Nowadays, we only display the error message to stderr so calling fprintf(stderr, ...) is much simpler. Signed-off-by: Olivier Gayot --- src/general.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/general.c b/src/general.c index f299a2b..2424cc6 100644 --- a/src/general.c +++ b/src/general.c @@ -51,12 +51,10 @@ char *skip_character(char *input, char character, int amount) { * */ void die(const char *fmt, ...) { - char buffer[512]; va_list ap; va_start(ap, fmt); - (void)vsnprintf(buffer, sizeof(buffer), fmt, ap); + (void)vfprintf(stderr, fmt, ap); va_end(ap); - fprintf(stderr, "%s", buffer); exit(EXIT_FAILURE); } From 598b76cc5332e196518a9ec6d722ebaeb3781a9c Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Tue, 12 Jun 2018 09:41:44 +0200 Subject: [PATCH 2/7] Make sure the arguments passed to printf/die(...) match the format Signed-off-by: Olivier Gayot --- i3status.c | 4 ++-- include/i3status.h | 3 ++- src/print_volume.c | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/i3status.c b/i3status.c index 6461d6b..316159c 100644 --- a/i3status.c +++ b/i3status.c @@ -106,7 +106,7 @@ static bool path_exists(const char *path) { static void *scalloc(size_t size) { void *result = calloc(size, 1); - exit_if_null(result, "Error: out of memory (calloc(%zd))\n", size); + exit_if_null(result, "Error: out of memory (calloc(%zu))\n", size); return result; } @@ -142,7 +142,7 @@ static int parse_min_width(cfg_t *context, cfg_opt_t *option, const char *value, long num = strtol(value, &end, 10); if (num < 0) - die("Invalid min_width attribute found in section %s, line %d: %d\n" + die("Invalid min_width attribute found in section %s, line %d: %ld\n" "Expected positive integer or string\n", context->name, context->line, num); else if (num == LONG_MIN || num == LONG_MAX || (end && *end != '\0')) diff --git a/include/i3status.h b/include/i3status.h index fe9206e..7bedfed 100644 --- a/include/i3status.h +++ b/include/i3status.h @@ -186,7 +186,8 @@ char *sstrdup(const char *str); /* src/general.c */ char *skip_character(char *input, char character, int amount); -void die(const char *fmt, ...); + +void die(const char *fmt, ...) __attribute__((format(printf, 1, 2), noreturn)); bool slurp(const char *filename, char *destination, int size); /* src/output.c */ diff --git a/src/print_volume.c b/src/print_volume.c index be6a1d7..e28a132 100644 --- a/src/print_volume.c +++ b/src/print_volume.c @@ -151,7 +151,7 @@ void print_volume(yajl_gen json_gen, char *buffer, const char *fmt, const char * snd_mixer_selem_id_set_index(sid, mixer_idx); snd_mixer_selem_id_set_name(sid, mixer); if (!(elem = snd_mixer_find_selem(m, sid))) { - fprintf(stderr, "i3status: ALSA: Cannot find mixer %s (index %i)\n", + fprintf(stderr, "i3status: ALSA: Cannot find mixer %s (index %u)\n", snd_mixer_selem_id_get_name(sid), snd_mixer_selem_id_get_index(sid)); snd_mixer_close(m); snd_mixer_selem_id_free(sid); From a6dd14c8c62e7e771ca76e9a5d897e8c90d1882a Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Wed, 13 Jun 2018 09:53:50 +0200 Subject: [PATCH 3/7] Avoid assigning a new value to a var before using the old value Signed-off-by: Olivier Gayot --- src/print_mem.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/print_mem.c b/src/print_mem.c index 46523d6..a37fa29 100644 --- a/src/print_mem.c +++ b/src/print_mem.c @@ -39,10 +39,9 @@ static int print_bytes_human(char *outwalk, uint64_t bytes) { * */ static long memory_absolute(const long mem_total, const char *size) { - long mem_absolute = -1; char *endptr = NULL; - mem_absolute = strtol(size, &endptr, 10); + long mem_absolute = strtol(size, &endptr, 10); if (endptr) { while (endptr[0] != '\0' && isspace(endptr[0])) From 95c068358acb303ab01ab8dc372f893aec5fe1b0 Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Tue, 19 Jun 2018 19:25:15 +0200 Subject: [PATCH 4/7] Fix use of undefined macro __OpenBSD__ Compiling on Linux with -Wundef produces the following warning: warning: "__OpenBSD__" is not defined, evaluates to 0 [-Wundef] Signed-off-by: Olivier Gayot --- src/print_disk_info.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/print_disk_info.c b/src/print_disk_info.c index fb73480..770e718 100644 --- a/src/print_disk_info.c +++ b/src/print_disk_info.c @@ -7,7 +7,7 @@ #include #include #include -#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || (__OpenBSD__) || defined(__DragonFly__) || defined(__APPLE__) +#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__OpenBSD__) || defined(__DragonFly__) || defined(__APPLE__) #include #include #elif defined(__NetBSD__) From c64195d14728aa82e280d022e9f7ceff71cfc6c1 Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Wed, 20 Jun 2018 11:42:59 +0200 Subject: [PATCH 5/7] Fix invalid handling of glob() errors on Linux The manual of glob(3) says that the function returns 0 on successful completion. Any other integer value should be considered an error, not only negative integers. In practice, *BSD systems use negative values but Linux uses positive integers. Signed-off-by: Olivier Gayot --- src/print_cpu_temperature.c | 2 +- src/process_runs.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/print_cpu_temperature.c b/src/print_cpu_temperature.c index 431664e..feae3ec 100644 --- a/src/print_cpu_temperature.c +++ b/src/print_cpu_temperature.c @@ -223,7 +223,7 @@ void print_cpu_temperature_info(yajl_gen json_gen, char *buffer, int zone, const asprintf(&thermal_zone, THERMAL_ZONE, zone); else { static glob_t globbuf; - if (glob(path, GLOB_NOCHECK | GLOB_TILDE, NULL, &globbuf) < 0) + if (glob(path, GLOB_NOCHECK | GLOB_TILDE, NULL, &globbuf) != 0) die("glob() failed\n"); if (globbuf.gl_pathc == 0) { /* No glob matches, the specified path does not contain a wildcard. */ diff --git a/src/process_runs.c b/src/process_runs.c index b5e8f11..da2dba1 100644 --- a/src/process_runs.c +++ b/src/process_runs.c @@ -24,7 +24,7 @@ bool process_runs(const char *path) { static glob_t globbuf; memset(pidbuf, 0, sizeof(pidbuf)); - if (glob(path, GLOB_NOCHECK | GLOB_TILDE, NULL, &globbuf) < 0) + if (glob(path, GLOB_NOCHECK | GLOB_TILDE, NULL, &globbuf) != 0) die("glob() failed\n"); if (globbuf.gl_pathc == 0) { /* No glob matches, the specified path does not contain a wildcard. */ From 445b1925e3d2e239740d7765e5892d0671174e8e Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Wed, 20 Jun 2018 11:58:10 +0200 Subject: [PATCH 6/7] Fix potential memory leak on Linux The function slurp_all_batteries(), on Linux, allocates memory dynamically Signed-off-by: Olivier Gayot --- src/print_battery_info.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/print_battery_info.c b/src/print_battery_info.c index 7a462f7..72c2d6f 100644 --- a/src/print_battery_info.c +++ b/src/print_battery_info.c @@ -450,8 +450,11 @@ static bool slurp_all_batteries(struct battery_info *batt_info, yajl_gen json_ge .present_rate = 0, .status = CS_UNKNOWN, }; - if (!slurp_battery_info(&batt_buf, json_gen, buffer, i, globbuf.gl_pathv[i], format_down)) + if (!slurp_battery_info(&batt_buf, json_gen, buffer, i, globbuf.gl_pathv[i], format_down)) { + globfree(&globbuf); + free(globpath); return false; + } is_found = true; add_battery_info(batt_info, &batt_buf); From c221b4d331d46d07fe24afc78e4eeb021556d013 Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Wed, 20 Jun 2018 13:41:59 +0200 Subject: [PATCH 7/7] Prevent potential crash if glob() fails Calling globfree(NULL) is undefined behaviour. In Linux (glibc), it results in a segmentation fault. It is also undefined behaviour to call globfree(&pglob) if a previous call to glob(&pglob) returned an error. Signed-off-by: Olivier Gayot --- src/print_battery_info.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/print_battery_info.c b/src/print_battery_info.c index 72c2d6f..04b5a25 100644 --- a/src/print_battery_info.c +++ b/src/print_battery_info.c @@ -459,8 +459,8 @@ static bool slurp_all_batteries(struct battery_info *batt_info, yajl_gen json_ge is_found = true; add_battery_info(batt_info, &batt_buf); } + globfree(&globbuf); } - globfree(&globbuf); free(globpath); if (!is_found) {