clean up path/file handling
authorNathan Lynch <ntl@pobox.com>
Tue, 21 Oct 2008 12:14:15 +0000 (07:14 -0500)
committerNathan Lynch <ntl@pobox.com>
Tue, 21 Oct 2008 12:14:15 +0000 (07:14 -0500)
Stop constructing sysfs paths "by hand" in various functions,
introducing some utility functions (sysfs_opendir, sysfs_file_getline,
etc) that internally use the context's sysfs root path to access the
appropriate filesystem object.  Cache a fd for the sysfs root
directory in the context during initialization so we can openat() the
paths provided by callers.

lib/topology.c

index 7406bc09e42d046707adb5f3e43ec86f0f6e2c2c..71e69f6c0d4c517ea41642e0c5f4ecc5ca304125 100644 (file)
@@ -23,6 +23,7 @@
 #include <dirent.h>
 #include <sched.h>
 #include <search.h>
+#include <stdarg.h>
 #include <stdbool.h>
 #include <stdlib.h>
 #include <stdio.h>
@@ -159,6 +160,7 @@ struct topo_procent {
  */
 struct topo_context {
        const char *sysfs_root;
+       int sysfs_root_fd;
        size_t cpu_set_t_size; /* for use with glibc accessors that
                                * take a setsize, e.g. CPU_SET_S() */
        struct topo_procent *list; /* global list of entities */
@@ -542,36 +544,116 @@ static int hash_device(struct topo_context *ctx, struct topo_device *dev)
        return htable_insert(&ctx->device_htable, dev->hash_key, dev);
 }
 
+static void buf_remove_newline(char *buf)
+{
+       char *newline;
+
+       if (!buf)
+               return;
+
+       newline = strchr(buf, '\n');
+
+       if (newline)
+               *newline = '\0';
+}
+
+static int sysfs_open_vargs(struct topo_context *ctx, const char *fmt, va_list vargs)
+{
+       char path[PATH_MAX];
+       int sysfs_root_fd;
+       int path_fd;
+
+       sysfs_root_fd = ctx->sysfs_root_fd;
+
+       assert(sysfs_root_fd != -1);
+
+       vsnprintf(path, sizeof(path), fmt, vargs);
+
+       path_fd = openat(sysfs_root_fd, path, O_RDONLY | O_CLOEXEC);
+
+       return path_fd;
+}
+
+static DIR *sysfs_opendir(struct topo_context *ctx, const char *fmt, ...)
+       __attribute__((format(printf, 2, 3)));
+static DIR *sysfs_opendir(struct topo_context *ctx, const char *fmt, ...)
+{
+       DIR *dir;
+       int dir_fd;
+       va_list vargs;
+
+       va_start(vargs, fmt);
+       dir_fd = sysfs_open_vargs(ctx, fmt, vargs);
+       va_end(vargs);
+
+       if (dir_fd == -1)
+               return NULL;
+
+       dir = fdopendir(dir_fd);
+       if (!dir)
+               close(dir_fd);
+
+       return dir;
+}
+
+static char *sysfs_file_getline(struct topo_context *ctx, const char *fmt, ...)
+       __attribute__((format(printf, 2, 3)));
+static char *sysfs_file_getline(struct topo_context *ctx, const char *fmt, ...)
+{
+       char *buf = NULL;
+       va_list vargs;
+       size_t size;
+       FILE *file;
+       int fd;
+
+       va_start(vargs, fmt);
+       fd = sysfs_open_vargs(ctx, fmt, vargs);
+       va_end(vargs);
+
+       if (fd == -1)
+               return NULL;
+
+       file = fdopen(fd, "r");
+       if (!file)
+               goto out;
+       fd = -1;
+
+       getline(&buf, &size, file);
+       buf_remove_newline(buf);
+
+       fclose(file);
+out:
+       if (fd != -1)
+               close(fd);
+       return buf;
+}
+
 /* Return true if:
  * - /sys/devices/system/cpu/cpu$cpu/online exists and has value 1, or
  * - /sys/devices/system/cpu/cpu$cpu/online does not exist (e.g. x86 boot cpu,
  *   or kernel does not support cpu hotplug)
  * Return false if /sys/devices/system/cpu/cpu$cpu/online has value 0
  */
-static int sysfs_cpu_is_online(const char *sysfs, unsigned int cpu)
+static bool sysfs_cpu_is_online(struct topo_context *ctx, unsigned int cpu)
 {
-       char path[PATH_MAX];
-       int online = 1;
+       int online = true;
        char *buf;
 
-       sprintf(path, "%s/devices/system/cpu/cpu%d/online", sysfs, cpu);
-
-       buf = slurp_text_file(path);
-       if (!buf) /* assume online on error */
+       buf = sysfs_file_getline(ctx, "devices/system/cpu/cpu%d/online", cpu);
+       if (!buf)
                goto out;
 
-       if (sscanf(buf, "%d", &online) != 1)
-               online = 1;
+       if (!strcmp("0", buf))
+               online = false;
 out:
        free(buf);
        return online;
 }
 
-static size_t sysfs_probe_cpumask_size(const char *sysfs)
+static size_t sysfs_probe_cpumask_size(struct topo_context *ctx)
 {
        DIR *dir;
        struct dirent64 *dirent;
-       char path[PATH_MAX];
        int max_cpu_id = 0;
 
        /* This must not be called on systems that don't support
@@ -579,9 +661,7 @@ static size_t sysfs_probe_cpumask_size(const char *sysfs)
         */
        assert(CPU_ALLOC_SIZE(CPU_SETSIZE + 1) > sizeof(cpu_set_t));
 
-       snprintf(path, sizeof(path), "%s/devices/system/cpu", sysfs);
-
-       dir = opendir(path);
+       dir = sysfs_opendir(ctx, "devices/system/cpu");
        if (!dir)
                goto err;
 
@@ -652,7 +732,7 @@ err:
  * sched_getaffinity size, and LIBTOPOLOGY_CPUMASK_OVERRIDE is not
  * set, return error (zero).
  */
-static size_t probe_cpumask_size(const char *sysfs)
+static size_t probe_cpumask_size(struct topo_context *ctx)
 {
        size_t sched_size;
        size_t sysfs_size;
@@ -667,7 +747,7 @@ static size_t probe_cpumask_size(const char *sysfs)
                goto err;
        ret = sched_size;
 
-       sysfs_size = sysfs_probe_cpumask_size(sysfs);
+       sysfs_size = sysfs_probe_cpumask_size(ctx);
        if (sysfs_size > sched_size) {
                if (!getenv("LIBTOPOLOGY_CPUMASK_OVERRIDE"))
                        goto err;
@@ -679,17 +759,13 @@ err:
        return 0;
 }
 
-static int sysfs_count_caches(const char *sysfs, int cpu)
+static int sysfs_count_caches(struct topo_context *ctx, int cpu)
 {
        struct dirent64 *dirent;
-       char path[PATH_MAX];
        int count = 0;
        DIR *dir;
 
-       snprintf(path, sizeof(path), "%s/devices/system/cpu/cpu%d/cache",
-                sysfs, cpu);
-
-       dir = opendir(path);
+       dir = sysfs_opendir(ctx, "devices/system/cpu/cpu%d/cache", cpu);
        if (!dir)
                goto out;
 
@@ -710,16 +786,6 @@ out:
        return count;
 }
 
-static void buf_remove_newline(char *buf)
-{
-       char *newline;
-
-       newline = strchr(buf, '\n');
-
-       if (newline)
-               *newline = '\0';
-}
-
 static char *sysfs_get_attr_value(int dirfd, const char *attr_path)
 {
        char *line = NULL;
@@ -780,21 +846,15 @@ err:
 static int __get_cache_info(struct topo_device *cache, int cpu, int index)
 {
        size_t cpumask_size;
-       char path[PATH_MAX];
-       const char *sysfs;
        struct attr *attr;
        DIR *dir;
        int fd;
 
        cpumask_size = cache->context->cpu_set_t_size;
 
-       sysfs = cache->context->sysfs_root;
-
-       snprintf(path, sizeof(path),
-                "%s/devices/system/cpu/cpu%d/cache/index%d",
-                sysfs, cpu, index);
-
-       dir = opendir(path);
+       dir = sysfs_opendir(cache->context,
+                           "devices/system/cpu/cpu%d/cache/index%d",
+                           cpu, index);
        if (!dir)
                goto err;
 
@@ -878,7 +938,7 @@ static int get_cache_info(struct topo_procent *thread)
 
        ctx = thread->context;
 
-       nr_caches = sysfs_count_caches(ctx->sysfs_root, thread->id);
+       nr_caches = sysfs_count_caches(ctx, thread->id);
        if (nr_caches == 0)
                return 0;
 
@@ -915,17 +975,13 @@ err:
        return 1;
 }
 
-static char *sysfs_cpu_thread_siblings(const char *sysfs, int cpu_id)
+static char *sysfs_cpu_thread_siblings(struct topo_context *ctx, int cpu_id)
 {
-       char path[PATH_MAX];
+       char *fmt = "devices/system/cpu/cpu%d/topology/thread_siblings";
        char *buf;
        int rc;
 
-       snprintf(path, sizeof(path),
-                "%s/devices/system/cpu/cpu%d/topology/thread_siblings",
-                sysfs, cpu_id);
-
-       buf = slurp_text_file(path);
+       buf = sysfs_file_getline(ctx, fmt, cpu_id);
        if (buf)
                return buf;
 
@@ -936,22 +992,18 @@ static char *sysfs_cpu_thread_siblings(const char *sysfs, int cpu_id)
        return buf;
 }
 
-static char *sysfs_cpu_core_siblings(const char *sysfs, int cpu_id)
+static char *sysfs_cpu_core_siblings(struct topo_context *ctx, int cpu_id)
 {
-       char path[PATH_MAX];
+       char *fmt = "devices/system/cpu/cpu%d/topology/core_siblings";
        char *buf;
 
-       snprintf(path, sizeof(path),
-                "%s/devices/system/cpu/cpu%d/topology/core_siblings",
-                sysfs, cpu_id);
-
-       buf = slurp_text_file(path);
+       buf = sysfs_file_getline(ctx, fmt, cpu_id);
        if (buf)
                return buf;
 
        /* thread_siblings must be a subset of core_siblings so assume
         * one core per package */
-       return sysfs_cpu_thread_siblings(sysfs, cpu_id);
+       return sysfs_cpu_thread_siblings(ctx, cpu_id);
 }
 
 static int do_one_cpu(struct topo_procent *node, int cpu_id)
@@ -965,7 +1017,7 @@ static int do_one_cpu(struct topo_procent *node, int cpu_id)
        ctx = node->context;
 
        /* package */
-       siblings = sysfs_cpu_core_siblings(ctx->sysfs_root, cpu_id);
+       siblings = sysfs_cpu_core_siblings(ctx, cpu_id);
        if (!siblings)
                goto err;
 
@@ -989,7 +1041,7 @@ static int do_one_cpu(struct topo_procent *node, int cpu_id)
        free(siblings);
 
        /* core */
-       siblings = sysfs_cpu_thread_siblings(ctx->sysfs_root, cpu_id);
+       siblings = sysfs_cpu_thread_siblings(ctx, cpu_id);
        if (!siblings)
                goto err;
 
@@ -1032,22 +1084,16 @@ static int do_node_cpus(struct topo_procent *node)
 {
        struct topo_context *ctx;
        struct dirent64 *dirent;
-       char path[PATH_MAX];
        DIR *dir;
 
        ctx = node->context;
 
-       snprintf(path, sizeof(path), "%s/devices/system/node/node%d",
-                ctx->sysfs_root, node->id);
-
        /* If we're "faking" node 0, use the cpu sysfs hierarchy */
-       dir = opendir(path);
+       dir = sysfs_opendir(ctx, "devices/system/node/node%d", node->id);
        if (!dir && node->id != 0) {
                goto err;
        } else if (!dir) {
-               snprintf(path, sizeof(path), "%s/devices/system/cpu",
-                        ctx->sysfs_root);
-               dir = opendir(path);
+               dir = sysfs_opendir(ctx, "devices/system/cpu");
                if (!dir)
                        goto err;
        }
@@ -1058,7 +1104,7 @@ static int do_node_cpus(struct topo_procent *node)
                if (sscanf(dirent->d_name, "cpu%d", &cpu_id) != 1)
                        continue;
 
-               if (!sysfs_cpu_is_online(ctx->sysfs_root, cpu_id))
+               if (!sysfs_cpu_is_online(ctx, cpu_id))
                        continue;
 
                if (do_one_cpu(node, cpu_id))
@@ -1093,7 +1139,6 @@ static int build_context(struct topo_context *ctx)
 {
        struct topo_procent *system;
        struct dirent64 *dirent;
-       char path[PATH_MAX];
        DIR *dir = NULL;
 
        system = alloc_init_topo_procent(ctx, NULL, 0);
@@ -1102,8 +1147,7 @@ static int build_context(struct topo_context *ctx)
 
        ctx->system = system;
 
-       snprintf(path, sizeof(path), "%s/devices/system/node", ctx->sysfs_root);
-       dir = opendir(path);
+       dir = sysfs_opendir(ctx, "devices/system/node");
        if (!dir) /* Non-numa system, treat as single node */
                return do_one_node(system, 0);
 
@@ -1156,6 +1200,10 @@ static void context_release(struct topo_context *ctx)
        htable_release(&ctx->package_htable);
        htable_release(&ctx->core_htable);
        htable_release(&ctx->device_htable);
+
+       if (ctx->sysfs_root_fd != -1)
+               close(ctx->sysfs_root_fd);
+
        free(ctx);
 }
 
@@ -1171,7 +1219,12 @@ static struct topo_context *new_context(void)
        if (!ctx->sysfs_root)
                ctx->sysfs_root = "/sys";
 
-       ctx->cpu_set_t_size = probe_cpumask_size(ctx->sysfs_root);
+       /* cache a fd for the sysfs root in the context during init */
+       ctx->sysfs_root_fd = open(ctx->sysfs_root, O_RDONLY | O_CLOEXEC);
+       if (ctx->sysfs_root_fd == -1)
+               goto err;
+
+       ctx->cpu_set_t_size = probe_cpumask_size(ctx);
        if (ctx->cpu_set_t_size == 0)
                goto err;
 
@@ -1184,10 +1237,13 @@ static struct topo_context *new_context(void)
        if (htable_init(&ctx->device_htable, 2))
                goto err;
 
-
        if (build_context(ctx))
                goto err;
 
+       /* close the sysfs root fd, no longer needed */
+       close(ctx->sysfs_root_fd);
+       ctx->sysfs_root_fd = -1;
+
        return ctx;
 
 err: