[lxc-devel] [PATCH v2] lxc-ls: check for ENOMEM and tweaking
Christian Brauner
christian.brauner at mailbox.org
Sat Jan 16 00:03:44 UTC 2016
If lxc_container_new() fails we check for ENOMEM and if so goto out. If ENOMEM
is not set we will simply continue. The same goes for the call to regcomp() but
instead of checking for ENOMEM we need to check for REG_ESPACE.
Tweaking: Since lxc-ls might have to gather a lot of containers and I don't
know if compilers will always optimize this let's move *some* variable
declarations outside of the loop when it does not hinder readability
Set ls_nesting to 0 initially. Otherwise users will always see nested
containers printed.
Signed-off-by: Christian Brauner <christian.brauner at mailbox.org>
---
Changelog:
2016-01-16T0100:
- free() grp_tmp when ls_new() fails to prevent memory leak.
- dynamically allocate lock_path instead of
lock_path[MAXPATHLEN]:
I don't know if realloc() will gain us much here. Instead we
simply call malloc() and free() it once the recursive function
returns.
- set ls_nesting = 0 to prevent always listing nesting
containers
---
src/lxc/lxc_ls.c | 55 ++++++++++++++++++++++++++++++++++---------------------
1 file changed, 34 insertions(+), 21 deletions(-)
diff --git a/src/lxc/lxc_ls.c b/src/lxc/lxc_ls.c
index a4ab9b6..31279fb 100644
--- a/src/lxc/lxc_ls.c
+++ b/src/lxc/lxc_ls.c
@@ -186,7 +186,7 @@ Options :\n\
-g --groups comma separated list of groups a container must have to be displayed\n",
.options = my_longopts,
.parser = my_parser,
- .ls_nesting = MAX_NESTLVL,
+ .ls_nesting = 0,
};
int main(int argc, char *argv[])
@@ -315,7 +315,7 @@ static int ls_get(struct ls **m, size_t *size, const struct lxc_arguments *args,
if (lvl > args->ls_nesting)
return 0;
- int num = 0, ret = -1;
+ int ret = -1;
char **containers = NULL;
/* If we, at some level of nesting, encounter a stopped container but
* want to retrieve nested containers we need to build an absolute path
@@ -331,6 +331,7 @@ static int ls_get(struct ls **m, size_t *size, const struct lxc_arguments *args,
if (!dir_exists(path))
goto out;
+ int num = 0;
/* Do not do more work than is necessary right from the start. */
if (args->ls_active || (args->ls_active && args->ls_frozen))
num = list_active_containers(path, &containers, NULL);
@@ -341,6 +342,8 @@ static int ls_get(struct ls **m, size_t *size, const struct lxc_arguments *args,
goto out;
}
+ char *tmp = NULL;
+ int check;
struct ls *l = NULL;
struct lxc_container *c = NULL;
size_t i;
@@ -350,17 +353,23 @@ static int ls_get(struct ls **m, size_t *size, const struct lxc_arguments *args,
/* Filter container names by regex the user gave us. */
if (args->ls_regex) {
regex_t preg;
- if (regcomp(&preg, args->ls_regex, REG_NOSUB | REG_EXTENDED) != 0)
+ check = regcomp(&preg, args->ls_regex, REG_NOSUB | REG_EXTENDED);
+ if (check == REG_ESPACE) /* we're out of memory */
+ goto out;
+ else if (check != 0)
continue;
- int rc = regexec(&preg, name, 0, NULL, 0);
+ check = regexec(&preg, name, 0, NULL, 0);
regfree(&preg);
- if (rc != 0)
+ if (check != 0)
continue;
}
+ errno = 0;
c = lxc_container_new(name, path);
- if (!c)
- continue;
+ if ((errno == ENOMEM) && !c)
+ goto out;
+ else if (!c)
+ continue;
if (!c->is_defined(c))
goto put_and_next;
@@ -390,8 +399,10 @@ static int ls_get(struct ls **m, size_t *size, const struct lxc_arguments *args,
/* Now it makes sense to allocate memory. */
l = ls_new(m, size);
- if (!l)
+ if (!l) {
+ free(grp_tmp);
goto put_and_next;
+ }
/* How deeply nested are we? */
l->nestlvl = lvl;
@@ -422,7 +433,7 @@ static int ls_get(struct ls **m, size_t *size, const struct lxc_arguments *args,
if (!l->state)
goto put_and_next;
- char *tmp = ls_get_config_item(c, "lxc.start.auto", running);
+ tmp = ls_get_config_item(c, "lxc.start.auto", running);
if (tmp)
l->autostart = atoi(tmp);
free(tmp);
@@ -451,11 +462,9 @@ static int ls_get(struct ls **m, size_t *size, const struct lxc_arguments *args,
* all other information we need. */
if (args->ls_nesting && running) {
struct wrapargs wargs = (struct wrapargs){.args = NULL};
-
- /* Open a socket so that the child can communicate with
- * us. */
- int ret = socketpair(PF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, wargs.pipefd);
- if (ret == -1)
+ /* Open a socket so that the child can communicate with us. */
+ check = socketpair(PF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, wargs.pipefd);
+ if (check == -1)
goto put_and_next;
/* Set the next nesting level. */
@@ -472,12 +481,12 @@ static int ls_get(struct ls **m, size_t *size, const struct lxc_arguments *args,
/* fork(): Attach to the namespace of the child and run
* ls_get() in it which is called in ls_get_wrapper(). */
- int status = c->attach(c, ls_get_wrapper, &wargs, &aopt, &out);
+ check = c->attach(c, ls_get_wrapper, &wargs, &aopt, &out);
/* close the socket */
close(wargs.pipefd[1]);
/* Retrieve all information we want from the child. */
- if (status == 0)
+ if (check == 0)
if (ls_deserialize(wargs.pipefd[0], m, size) == -1)
goto put_and_next;
@@ -512,20 +521,24 @@ static int ls_get(struct ls **m, size_t *size, const struct lxc_arguments *args,
/* We want to remove all locks we created under
* /run/lxc/lock so we create a string pointing us to
* the lock path for the current container. */
- char lock_path[MAXPATHLEN];
- int ret = snprintf(lock_path, MAXPATHLEN, "%s/lxc/lock/%s/%s", RUNTIME_PATH, path, name);
- if (ret < 0 || ret >= MAXPATHLEN)
+ size_t len = strlen(path) + strlen(name) + strlen(RUNTIME_PATH) + /* /+lxc+/+lock+/+/= */ 11 + 1;
+ char *lock_path = malloc(len);
+ if (!lock_path)
+ goto put_and_next;
+
+ check = snprintf(lock_path, len, "%s/lxc/lock/%s/%s", RUNTIME_PATH, path, name);
+ if (check < 0 || (size_t)check >= len)
goto put_and_next;
/* Remove the lock. */
lxc_rmdir_onedev(lock_path, NULL);
ls_get(m, size, args, lht, newpath, l->name, lvl + 1);
+ free(newpath);
/* Remove the lock. */
lxc_rmdir_onedev(lock_path, NULL);
-
- free(newpath);
+ free(lock_path);
}
put_and_next:
--
2.7.0
More information about the lxc-devel
mailing list