From 34a4071e998f327945993ea6e6cbcaa0292b4093 Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Thu, 2 Apr 2015 13:43:18 +0200 Subject: [PATCH] bootchart: clean up control flow logic Don't blindly exit() from random functions, but return a proper error and upchain error conditions. squash! bootchart: clean up control flow logic When pread() returns "0", it's a read failure, so don't make the caller think log_sample() was successful, return meaningful error code instead of 0. --- src/bootchart/bootchart.c | 20 +++++++++------ src/bootchart/store.c | 53 ++++++++++++++++++--------------------- src/bootchart/store.h | 2 +- 3 files changed, 39 insertions(+), 36 deletions(-) diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c index e4ade7e2f..1024f78fa 100644 --- a/src/bootchart/bootchart.c +++ b/src/bootchart/bootchart.c @@ -328,8 +328,11 @@ int main(int argc, char *argv[]) { parse_conf(); r = parse_argv(argc, argv); - if (r <= 0) - return r == 0 ? EXIT_SUCCESS : EXIT_FAILURE; + if (r < 0) + return EXIT_FAILURE; + + if (r == 0) + return EXIT_SUCCESS; /* * If the kernel executed us through init=/usr/lib/systemd/systemd-bootchart, then @@ -367,7 +370,7 @@ int main(int argc, char *argv[]) { log_error("Failed to setup graph start time.\n\n" "The system uptime probably includes time that the system was suspended. " "Use --rel to bypass this issue."); - exit (EXIT_FAILURE); + return EXIT_FAILURE; } has_procfs = access("/proc/vmstat", F_OK) == 0; @@ -401,11 +404,14 @@ int main(int argc, char *argv[]) { parse_env_file("/usr/lib/os-release", NEWLINE, "PRETTY_NAME", &build, NULL); } - if (has_procfs) - log_sample(samples, &sampledata); - else + if (has_procfs) { + r = log_sample(samples, &sampledata); + if (r < 0) + return EXIT_FAILURE; + } else { /* wait for /proc to become available, discarding samples */ has_procfs = access("/proc/vmstat", F_OK) == 0; + } sample_stop = gettime_ns(); @@ -432,7 +438,7 @@ int main(int argc, char *argv[]) { break; } log_error_errno(errno, "nanosleep() failed: %m"); - exit(EXIT_FAILURE); + return EXIT_FAILURE; } } else { overrun++; diff --git a/src/bootchart/store.c b/src/bootchart/store.c index 0663e100e..53b827fe5 100644 --- a/src/bootchart/store.c +++ b/src/bootchart/store.c @@ -107,7 +107,7 @@ static int pid_cmdline_strscpy(char *buffer, size_t buf_len, int pid) { return 0; } -void log_sample(int sample, struct list_sample_data **ptr) { +int log_sample(int sample, struct list_sample_data **ptr) { static int vmstat = -1; static int schedstat = -1; char buf[4096]; @@ -134,7 +134,7 @@ void log_sample(int sample, struct list_sample_data **ptr) { /* find all processes */ proc = opendir("/proc"); if (!proc) - return; + return -errno; procfd = dirfd(proc); } else { rewinddir(proc); @@ -149,9 +149,10 @@ void log_sample(int sample, struct list_sample_data **ptr) { n = pread(vmstat, buf, sizeof(buf) - 1, 0); if (n <= 0) { - close(vmstat); - vmstat = -1; - return; + vmstat = safe_close(vmstat); + if (n < 0) + return -errno; + return -ENODATA; } buf[n] = '\0'; @@ -174,17 +175,16 @@ vmstat_next: if (schedstat < 0) { /* overall CPU utilization */ schedstat = openat(procfd, "schedstat", O_RDONLY); - if (schedstat == -1) { - log_error_errno(errno, "Failed to open /proc/schedstat (requires CONFIG_SCHEDSTATS=y in kernel config): %m"); - exit(EXIT_FAILURE); - } + if (schedstat == -1) + return log_error_errno(errno, "Failed to open /proc/schedstat (requires CONFIG_SCHEDSTATS=y in kernel config): %m"); } n = pread(schedstat, buf, sizeof(buf) - 1, 0); if (n <= 0) { - close(schedstat); - schedstat = -1; - return; + schedstat = safe_close(schedstat); + if (n < 0) + return -errno; + return -ENODATA; } buf[n] = '\0'; @@ -215,10 +215,8 @@ schedstat_next: if (arg_entropy) { if (e_fd < 0) { e_fd = openat(procfd, "sys/kernel/random/entropy_avail", O_RDONLY); - if (e_fd == -1) { - log_error_errno(errno, "Failed to open /proc/sys/kernel/random/entropy_avail: %m"); - exit(EXIT_FAILURE); - } + if (e_fd == -1) + return log_error_errno(errno, "Failed to open /proc/sys/kernel/random/entropy_avail: %m"); } n = pread(e_fd, buf, sizeof(buf) - 1, 0); @@ -259,20 +257,18 @@ schedstat_next: int r; ps->next_ps = new0(struct ps_struct, 1); - if (!ps->next_ps) { - log_oom(); - exit (EXIT_FAILURE); - } + if (!ps->next_ps) + return log_oom(); + ps = ps->next_ps; ps->pid = pid; ps->sched = -1; ps->schedstat = -1; ps->sample = new0(struct ps_sched_struct, 1); - if (!ps->sample) { - log_oom(); - exit (EXIT_FAILURE); - } + if (!ps->sample) + return log_oom(); + ps->sample->sampledata = sampledata; pscount++; @@ -414,10 +410,9 @@ schedstat_next: continue; ps->sample->next = new0(struct ps_sched_struct, 1); - if (!ps->sample->next) { - log_oom(); - exit(EXIT_FAILURE); - } + if (!ps->sample->next) + return log_oom(); + ps->sample->next->prev = ps->sample; ps->sample = ps->sample->next; ps->last = ps->sample; @@ -528,4 +523,6 @@ catch_rename: pid_cmdline_strscpy(ps->name, sizeof(ps->name), pid); } } + + return 0; } diff --git a/src/bootchart/store.h b/src/bootchart/store.h index f211b6f53..8cb319275 100644 --- a/src/bootchart/store.h +++ b/src/bootchart/store.h @@ -32,4 +32,4 @@ extern int procfd; double gettime_ns(void); void log_uptime(void); -void log_sample(int sample, struct list_sample_data **ptr); +int log_sample(int sample, struct list_sample_data **ptr); -- 2.30.2