bootchart: clean up sysfd and proc handling

Retrieve the handle to procfs in main(), and pass it functions
that need it. Kill the global variables.

Also, refactor lots of code in svg_title(). There's no need to access any
global variables from there either, and we really should return proper
errors from there as well.

diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c
index 1024f78fa649473e0cc66ad81ede68982b0d306e..8cb1ee69211cf2bd1ce123ba150be8f525682a09 100644 (file)
--- a/src/bootchart/bootchart.c
+++ b/src/bootchart/bootchart.c
@@ -67,7 +67,6 @@ double interval;
 FILE *of = NULL;
 int overrun = 0;
 static int exiting = 0;
-int sysfd=-1;
 
 #define DEFAULT_SAMPLES_LEN 500
 #define DEFAULT_HZ 25.0
@@ -314,6 +313,8 @@ static void do_journal_append(char *file) {
 
 int main(int argc, char *argv[]) {
         _cleanup_free_ char *build = NULL;
+        _cleanup_close_ int sysfd = -1;
+        _cleanup_closedir_ DIR *proc = NULL;
         struct sigaction sig = {
                 .sa_handler = signal_handler,
         };
@@ -323,7 +324,6 @@ int main(int argc, char *argv[]) {
         time_t t = 0;
         int r;
         struct rlimit rlim;
-        bool has_procfs = false;
 
         parse_conf();
 
@@ -373,8 +373,6 @@ int main(int argc, char *argv[]) {
                 return EXIT_FAILURE;
         }
 
-        has_procfs = access("/proc/vmstat", F_OK) == 0;
-
         LIST_HEAD_INIT(head);
 
         /* main program loop */
@@ -404,13 +402,16 @@ int main(int argc, char *argv[]) {
                                 parse_env_file("/usr/lib/os-release", NEWLINE, "PRETTY_NAME", &build, NULL);
                 }
 
-                if (has_procfs) {
-                        r = log_sample(samples, &sampledata);
+                if (proc)
+                        rewinddir(proc);
+                else
+                        proc = opendir("/proc");
+
+                /* wait for /proc to become available, discarding samples */
+                if (proc) {
+                        r = log_sample(proc, 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();
@@ -470,23 +471,23 @@ int main(int argc, char *argv[]) {
         }
 
         if (!of) {
-                fprintf(stderr, "opening output file '%s': %m\n", output_file);
-                exit (EXIT_FAILURE);
+                log_error("Error opening output file '%s': %m\n", output_file);
+                return EXIT_FAILURE;
         }
 
-        svg_do(strna(build));
+        r = svg_do(strna(build));
+        if (r < 0) {
+                log_error_errno(r, "Error generating svg file: %m\n");
+                return EXIT_FAILURE;
+        }
 
-        fprintf(stderr, "systemd-bootchart wrote %s\n", output_file);
+        log_info("systemd-bootchart wrote %s\n", output_file);
 
         do_journal_append(output_file);
 
         if (of)
                 fclose(of);
 
-        closedir(proc);
-        if (sysfd >= 0)
-                close(sysfd);
-
         /* nitpic cleanups */
         ps = ps_first->next_ps;
         while (ps->next_ps) {

diff --git a/src/bootchart/bootchart.h b/src/bootchart/bootchart.h
index e4dbdd921d5b75da03e340a9f0c8082ded1d6ce2..6e83a99a2b555f0f77ae8c32427abd84a16561ed 100644 (file)
--- a/src/bootchart/bootchart.h
+++ b/src/bootchart/bootchart.h
@@ -132,4 +132,3 @@ extern char arg_output_path[PATH_MAX];
 extern char arg_init_path[PATH_MAX];
 
 extern FILE *of;
-extern int sysfd;

diff --git a/src/bootchart/store.c b/src/bootchart/store.c
index 53b827fe59dcc8f45917951b7e6b7e4b78a72e92..612510408454439b7974437e4a43c55767a52117 100644 (file)
--- a/src/bootchart/store.c
+++ b/src/bootchart/store.c
@@ -45,8 +45,6 @@
  */
 static char smaps_buf[4096];
 static int skip = 0;
-DIR *proc;
-int procfd = -1;
 
 double gettime_ns(void) {
         struct timespec n;
@@ -86,7 +84,7 @@ static char *bufgetline(char *buf) {
         return c;
 }
 
-static int pid_cmdline_strscpy(char *buffer, size_t buf_len, int pid) {
+static int pid_cmdline_strscpy(int procfd, char *buffer, size_t buf_len, int pid) {
         char filename[PATH_MAX];
         _cleanup_close_ int fd=-1;
         ssize_t n;
@@ -107,7 +105,7 @@ static int pid_cmdline_strscpy(char *buffer, size_t buf_len, int pid) {
         return 0;
 }
 
-int log_sample(int sample, struct list_sample_data **ptr) {
+int log_sample(DIR *proc, int sample, struct list_sample_data **ptr) {
         static int vmstat = -1;
         static int schedstat = -1;
         char buf[4096];
@@ -126,19 +124,13 @@ int log_sample(int sample, struct list_sample_data **ptr) {
         int fd;
         struct list_sample_data *sampledata;
         struct ps_sched_struct *ps_prev = NULL;
+        int procfd;
 
         sampledata = *ptr;
 
-        /* all the per-process stuff goes here */
-        if (!proc) {
-                /* find all processes */
-                proc = opendir("/proc");
-                if (!proc)
-                        return -errno;
-                procfd = dirfd(proc);
-        } else {
-                rewinddir(proc);
-        }
+        procfd = dirfd(proc);
+        if (procfd < 0)
+                return -errno;
 
         if (vmstat < 0) {
                 /* block stuff */
@@ -301,7 +293,7 @@ schedstat_next:
 
                         /* cmdline */
                         if (arg_show_cmdline)
-                                pid_cmdline_strscpy(ps->name, sizeof(ps->name), pid);
+                                pid_cmdline_strscpy(procfd, ps->name, sizeof(ps->name), pid);
 
                         /* discard line 2 */
                         m = bufgetline(buf);
@@ -520,7 +512,7 @@ catch_rename:
 
                         /* cmdline */
                         if (arg_show_cmdline)
-                                pid_cmdline_strscpy(ps->name, sizeof(ps->name), pid);
+                                pid_cmdline_strscpy(procfd, ps->name, sizeof(ps->name), pid);
                 }
         }
 

diff --git a/src/bootchart/store.h b/src/bootchart/store.h
index 8cb319275ccf795b34599364d8ed3a0ade9d3389..bc54f4f63186c7efd574f7910b4deab55c554140 100644 (file)
--- a/src/bootchart/store.h
+++ b/src/bootchart/store.h
@@ -27,9 +27,6 @@
 #include <dirent.h>
 #include "bootchart.h"
 
-extern DIR *proc;
-extern int procfd;
-
 double gettime_ns(void);
 void log_uptime(void);
-int log_sample(int sample, struct list_sample_data **ptr);
+int log_sample(DIR *proc, int sample, struct list_sample_data **ptr);

diff --git a/src/bootchart/svg.c b/src/bootchart/svg.c
index 8c8fab941d9027f62665cc0f5b4990a60ac211ff..b031090d2ff8e3721ceac120ca4d78027057054d 100644 (file)
--- a/src/bootchart/svg.c
+++ b/src/bootchart/svg.c
@@ -31,6 +31,7 @@
 #include <fcntl.h>
 
 #include "util.h"
+#include "fileio.h"
 #include "macro.h"
 #include "store.h"
 #include "svg.h"
@@ -149,54 +150,47 @@ static void svg_header(void) {
         svg("    ]]>\n   </style>\n</defs>\n\n");
 }
 
-static void svg_title(const char *build) {
-        char cmdline[256] = "";
-        char filename[PATH_MAX];
-        char buf[256];
-        char rootbdev[16] = "Unknown";
-        char model[256] = "Unknown";
+static int svg_title(const char *build) {
+        _cleanup_free_ char *cmdline = NULL;
+        _cleanup_free_ char *model = NULL;
+        _cleanup_free_ char *buf = NULL;
         char date[256] = "Unknown";
-        char cpu[256] = "Unknown";
+        char *cpu;
         char *c;
-        FILE *f;
         time_t t;
-        int fd, r;
+        int r;
         struct utsname uts;
 
-        /* grab /proc/cmdline */
-        fd = openat(procfd, "cmdline", O_RDONLY);
-        f = fdopen(fd, "r");
-        if (f) {
-                if (!fgets(cmdline, 255, f))
-                        sprintf(cmdline, "Unknown");
-                fclose(f);
-        } else {
-                if (fd >= 0)
-                        close(fd);
+        r = read_one_line_file("/proc/cmdline", &cmdline);
+        if (r < 0) {
+                log_error_errno(r, "Unable to read cmdline: %m\n");
+                return r;
         }
 
         /* extract root fs so we can find disk model name in sysfs */
         /* FIXME: this works only in the simple case */
         c = strstr(cmdline, "root=/dev/");
         if (c) {
-                strncpy(rootbdev, &c[10], 3);
+                char rootbdev[4];
+                char filename[32];
+
+                strncpy(rootbdev, &c[10], sizeof(rootbdev) - 1);
                 rootbdev[3] = '\0';
-                sprintf(filename, "block/%s/device/model", rootbdev);
-                fd = openat(sysfd, filename, O_RDONLY);
-                f = fdopen(fd, "r");
-                if (f) {
-                        if (!fgets(model, 255, f))
-                                log_error("Error reading disk model for %s: %m\n", rootbdev);
-                        fclose(f);
-                } else {
-                        if (fd >= 0)
-                                close(fd);
+                snprintf(filename, sizeof(filename), "/sys/block/%s/device/model", rootbdev);
+
+                r = read_one_line_file(filename, &model);
+                if (r < 0) {
+                        log_error("Error reading disk model for %s: %m\n", rootbdev);
+                        return r;
                 }
         }
 
         /* various utsname parameters */
-        if (uname(&uts))
+        r = uname(&uts);
+        if (r < 0) {
                 log_error("Error getting uname info\n");
+                return -errno;
+        }
 
         /* date */
         t = time(NULL);
@@ -204,21 +198,23 @@ static void svg_title(const char *build) {
         assert_se(r > 0);
 
         /* CPU type */
-        fd = openat(procfd, "cpuinfo", O_RDONLY);
-        f = fdopen(fd, "r");
-        if (f) {
-                while (fgets(buf, 255, f)) {
-                        if (strstr(buf, "model name")) {
-                                strncpy(cpu, &buf[13], 255);
-                                break;
-                        }
-                }
-                fclose(f);
-        } else {
-                if (fd >= 0)
-                        close(fd);
+        r = read_full_file("/proc/cpuinfo", &buf, NULL);
+        if (r < 0) {
+                log_error_errno(r, "Unable to read cpuinfo: %m\n");
+                return r;
+        }
+
+        cpu = strstr(buf, "model name");
+        if (!cpu) {
+                log_error("Unable to read module name from cpuinfo.\n");
+                return -ENOENT;
         }
 
+        cpu += 13;
+        c = strchr(cpu, '\n');
+        if (c)
+                *c = '\0';
+
         svg("<text class=\"t1\" x=\"0\" y=\"30\">Bootchart for %s - %s</text>\n",
             uts.nodename, date);
         svg("<text class=\"t2\" x=\"20\" y=\"50\">System: %s %s %s %s</text>\n",
@@ -241,6 +237,8 @@ static void svg_title(const char *build) {
         svg("</text>\n");
         svg("<text class=\"sec\" x=\"20\" y=\"155\">Graph data: %.03f samples/sec, recorded %i total, dropped %i samples, %i processes, %i filtered</text>\n",
             arg_hz, arg_samples_len, overrun, pscount, pfiltered);
+
+        return 0;
 }
 
 static void svg_graph_box(int height) {
@@ -1273,10 +1271,10 @@ static void svg_top_ten_pss(void) {
                     top[n]->pid);
 }
 
-void svg_do(const char *build) {
+int svg_do(const char *build) {
         struct ps_struct *ps;
         double offset = 7;
-        int c;
+        int r, c;
 
         memzero(&str, sizeof(str));
 
@@ -1334,9 +1332,12 @@ void svg_do(const char *build) {
         svg("</g>\n\n");
 
         svg("<g transform=\"translate(10,  0)\">\n");
-        svg_title(build);
+        r = svg_title(build);
         svg("</g>\n\n");
 
+        if (r < 0)
+                return r;
+
         svg("<g transform=\"translate(10,200)\">\n");
         svg_top_ten_cpu();
         svg("</g>\n\n");
@@ -1359,4 +1360,6 @@ void svg_do(const char *build) {
 
         /* svg footer */
         svg("\n</svg>\n");
+
+        return 0;
 }

diff --git a/src/bootchart/svg.h b/src/bootchart/svg.h
index df3a7bf8efd3097003ef49c3167e3a0f265e176d..f0fb1a7ce7da6af2ef39be1ab2c72e4a3dd7fb1a 100644 (file)
--- a/src/bootchart/svg.h
+++ b/src/bootchart/svg.h
@@ -24,4 +24,4 @@
   along with systemd; If not, see <http://www.gnu.org/licenses/>.
 ***/
 
-void svg_do(const char *build);
+int svg_do(const char *build);