[lxc-devel] [lxcfs/master] cleanups, deduplication and liblxcfstest installation
Blub on Github
lxc-bot at linuxcontainers.org
Fri Feb 5 12:36:31 UTC 2016
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 462 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160205/73f11b1f/attachment.bin>
-------------- next part --------------
From e390800e6f8140ac85ed9bda0f657f55e9203ede Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Fri, 5 Feb 2016 11:50:32 +0100
Subject: [PATCH 1/4] bindings: return value type fixup
Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
bindings.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/bindings.c b/bindings.c
index 4bb0631..5394b44 100644
--- a/bindings.c
+++ b/bindings.c
@@ -665,7 +665,7 @@ bool cgfs_list_children(const char *controller, const char *cgroup, char ***list
(*list)[0] = NULL;
if (!tmpc)
- return NULL;
+ return false;
/* basedir / tmpc / cgroup \0 */
len = strlen(basedir) + strlen(tmpc) + strlen(cgroup) + 3;
@@ -815,7 +815,7 @@ bool cgfs_list_keys(const char *controller, const char *cgroup, struct cgfs_file
*keys = NULL;
if (!tmpc)
- return NULL;
+ return false;
/* basedir / tmpc / cgroup \0 */
len = strlen(basedir) + strlen(tmpc) + strlen(cgroup) + 3;
From 620525f6c82955fc0a5ae62e1f51374d9a22f3cb Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Fri, 5 Feb 2016 12:10:15 +0100
Subject: [PATCH 2/4] bindings: even more concise must_strcat_pid
We already assume tmp[] is big enough when using an unsized
sprintf(), considering it contains a single pid number and
is 30 bytes we can assume it was also big enough to hold the
terminating null byte.
Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
bindings.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/bindings.c b/bindings.c
index 5394b44..6e6be6e 100644
--- a/bindings.c
+++ b/bindings.c
@@ -1052,9 +1052,8 @@ static void must_strcat_pid(char **src, size_t *sz, size_t *asz, pid_t pid)
*src = tmp;
*asz += BUF_RESERVE_SIZE;
}
- memcpy((*src) +*sz , tmp, tmplen);
+ memcpy((*src) +*sz , tmp, tmplen+1); /* include the \0 */
*sz += tmplen;
- (*src)[*sz] = '\0';
}
/*
From 9ef33e4f52d23e102c911f4658eaeb45aa70e616 Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Fri, 5 Feb 2016 12:30:17 +0100
Subject: [PATCH 3/4] tests: don't actually install liblxcfstest.so
Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
Makefile.am | 3 ++-
tests/test_reload.sh | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 734fa3b..c835d56 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -27,7 +27,8 @@ liblxcfstest_la_LDFLAGS = $(AM_CFLAGS) -shared \
noinst_HEADERS = bindings.h
sodir=$(libdir)
-lib_LTLIBRARIES = liblxcfs.la liblxcfstest.la
+lib_LTLIBRARIES = liblxcfs.la
+EXTRA_LTLIBRARIES = liblxcfstest.la
lxcfs_SOURCES = lxcfs.c
lxcfs_LDADD = liblxcfs.la -ldl
diff --git a/tests/test_reload.sh b/tests/test_reload.sh
index 4560ba2..e0fd605 100755
--- a/tests/test_reload.sh
+++ b/tests/test_reload.sh
@@ -37,7 +37,8 @@ cleanup() {
trap cleanup EXIT SIGHUP SIGINT SIGTERM
-( cd ${topdir}; DESTDIR=${installdir} make install )
+( cd ${topdir}; DESTDIR=${installdir} make install)
+( cd ${topdir}; make liblxcfstest.la && cp .libs/liblxcfstest.* "${libdir}" )
export LD_LIBRARY_PATH=${libdir}
${bindir}/lxcfs -p ${pidfile} ${testdir} &
From 0d91d28d92431f75b4e6fbbe5e9916eedc41eedc Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Fri, 5 Feb 2016 11:52:23 +0100
Subject: [PATCH 4/4] bindings: avoid allocating an unused buffer
cgfs_list_children() and cgfs_list_keys() follow the same
pattern with the differences being that one lists
directories, the other files, and that cgfs_list_children()
always allocates an empty list while cgfs_list_keys()
NULL-initializes the list.
Both have a case which returns an error after a list has
been allocated, and in both cases the cleanup code is
guarded with an if(list).
In both cases on success the caller assumes the list is
non-empty which is why cgfs_list_children() returned a list
with a terminating NULL-entry.
This deduplicates the iteration code into a function with a
flag for whether regular files or directories are of
interest and a callback to create the list element.
Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
bindings.c | 126 ++++++++++++++++++++-----------------------------------------
1 file changed, 41 insertions(+), 85 deletions(-)
diff --git a/bindings.c b/bindings.c
index 6e6be6e..c093a7e 100644
--- a/bindings.c
+++ b/bindings.c
@@ -649,21 +649,19 @@ FILE *open_pids_file(const char *controller, const char *cgroup)
return fopen(pathname, "w");
}
-bool cgfs_list_children(const char *controller, const char *cgroup, char ***list)
+static bool cgfs_iterate_cgroup(const char *controller, const char *cgroup, bool directories,
+ void ***list, size_t typesize,
+ void* (*iterator)(const char*, const char*, const char*))
{
size_t len;
char *dirname, *tmpc = find_mounted_controller(controller);
char pathname[MAXPATHLEN];
- size_t sz = 0, asz = BATCH_SIZE;
+ size_t sz = 0, asz = 0;
struct dirent dirent, *direntp;
DIR *dir;
int ret;
- do {
- *list = malloc(asz * sizeof(char *));
- } while (!*list);
- (*list)[0] = NULL;
-
+ *list = NULL;
if (!tmpc)
return false;
@@ -698,20 +696,19 @@ bool cgfs_list_children(const char *controller, const char *cgroup, char ***list
fprintf(stderr, "%s: failed to stat %s: %s\n", __func__, pathname, strerror(errno));
continue;
}
- if (!S_ISDIR(mystat.st_mode))
+ if ((!directories && !S_ISREG(mystat.st_mode)) ||
+ (directories && !S_ISDIR(mystat.st_mode)))
continue;
if (sz+2 >= asz) {
- char **tmp;
+ void **tmp;
asz += BATCH_SIZE;
do {
- tmp = realloc(*list, asz * sizeof(char *));
+ tmp = realloc(*list, asz * typesize);
} while (!tmp);
*list = tmp;
}
- do {
- (*list)[sz] = strdup(direntp->d_name);
- } while (!(*list)[sz]);
+ (*list)[sz] = (*iterator)(controller, cgroup, direntp->d_name);
(*list)[sz+1] = NULL;
sz++;
}
@@ -722,6 +719,20 @@ bool cgfs_list_children(const char *controller, const char *cgroup, char ***list
return true;
}
+static void *make_children_list_entry(const char *controller, const char *cgroup, const char *dir_entry)
+{
+ char *dup;
+ do {
+ dup = strdup(dir_entry);
+ } while (!dup);
+ return dup;
+}
+
+bool cgfs_list_children(const char *controller, const char *cgroup, char ***list)
+{
+ return cgfs_iterate_cgroup(controller, cgroup, true, (void***)list, sizeof(*list), &make_children_list_entry);
+}
+
void free_key(struct cgfs_files *k)
{
if (!k)
@@ -803,76 +814,19 @@ struct cgfs_files *cgfs_get_key(const char *controller, const char *cgroup, cons
return newkey;
}
-bool cgfs_list_keys(const char *controller, const char *cgroup, struct cgfs_files ***keys)
+static void *make_key_list_entry(const char *controller, const char *cgroup, const char *dir_entry)
{
- size_t len;
- char *dirname, *tmpc = find_mounted_controller(controller);
- char pathname[MAXPATHLEN];
- size_t sz = 0, asz = 0;
- struct dirent dirent, *direntp;
- DIR *dir;
- int ret;
-
- *keys = NULL;
- if (!tmpc)
- return false;
-
- /* basedir / tmpc / cgroup \0 */
- len = strlen(basedir) + strlen(tmpc) + strlen(cgroup) + 3;
- dirname = alloca(len);
- snprintf(dirname, len, "%s/%s/%s", basedir, tmpc, cgroup);
-
- dir = opendir(dirname);
- if (!dir)
- return false;
-
- while (!readdir_r(dir, &dirent, &direntp)) {
- struct stat mystat;
- int rc;
-
- if (!direntp)
- break;
-
- if (!strcmp(direntp->d_name, ".") ||
- !strcmp(direntp->d_name, ".."))
- continue;
-
- rc = snprintf(pathname, MAXPATHLEN, "%s/%s", dirname, direntp->d_name);
- if (rc < 0 || rc >= MAXPATHLEN) {
- fprintf(stderr, "%s: pathname too long under %s\n", __func__, dirname);
- continue;
- }
-
- ret = lstat(pathname, &mystat);
- if (ret) {
- fprintf(stderr, "%s: failed to stat %s: %s\n", __func__, pathname, strerror(errno));
- continue;
- }
- if (!S_ISREG(mystat.st_mode))
- continue;
-
- if (sz+2 >= asz) {
- struct cgfs_files **tmp;
- asz += BATCH_SIZE;
- do {
- tmp = realloc(*keys, asz * sizeof(struct cgfs_files *));
- } while (!tmp);
- *keys = tmp;
- }
- (*keys)[sz] = cgfs_get_key(controller, cgroup, direntp->d_name);
- (*keys)[sz+1] = NULL;
- if (!(*keys)[sz]) {
- fprintf(stderr, "%s: Error getting files under %s:%s\n",
- __func__, controller, cgroup);
- continue;
- }
- sz++;
+ struct cgfs_files *entry = cgfs_get_key(controller, cgroup, dir_entry);
+ if (!entry) {
+ fprintf(stderr, "%s: Error getting files under %s:%s\n",
+ __func__, controller, cgroup);
}
- if (closedir(dir) < 0) {
- fprintf(stderr, "%s: failed closedir for %s: %s\n", __func__, dirname, strerror(errno));
- return false;
- }
- return true;
+ return entry;
+}
+
+bool cgfs_list_keys(const char *controller, const char *cgroup, struct cgfs_files ***keys)
+{
+ return cgfs_iterate_cgroup(controller, cgroup, false, (void***)keys, sizeof(*keys), &make_key_list_entry);
}
bool is_child_cgroup(const char *controller, const char *cgroup, const char *f)
@@ -1695,10 +1649,12 @@ int cg_readdir(const char *path, void *buf, fuse_fill_dir_t filler, off_t offset
ret = 0;
goto out;
}
- for (i = 0; clist[i]; i++) {
- if (filler(buf, clist[i], NULL, 0) != 0) {
- ret = -EIO;
- goto out;
+ if (clist) {
+ for (i = 0; clist[i]; i++) {
+ if (filler(buf, clist[i], NULL, 0) != 0) {
+ ret = -EIO;
+ goto out;
+ }
}
}
ret = 0;
More information about the lxc-devel
mailing list