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.

diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c
index e4ade7e2f5006bb5be953d609bea70de0f772dae..1024f78fa649473e0cc66ad81ede68982b0d306e 100644 (file)
--- 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 0663e100e8dcc96488e245dc1254f1793fec15da..53b827fe59dcc8f45917951b7e6b7e4b78a72e92 100644 (file)
--- 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 f211b6f53b512782000389b6488356d3effc97d7..8cb319275ccf795b34599364d8ed3a0ade9d3389 100644 (file)
--- 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);