[lxc-devel] [PATCH] lxc-ls: check for ENOMEM and tweaking
Christian Brauner
christian.brauner at mailbox.org
Sat Jan 16 00:04:12 UTC 2016
Ignore.
On Sat, Jan 16, 2016 at 12:57:24AM +0100, Christian Brauner wrote:
> 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>
> ---
> 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