Quoting Christian Brauner (christianvanbrau...@gmail.com):
> - remove unused argument from ls_get()
> - Fix ls_has_all_groups() but leave the inefficient basic algorithm untouched
>   for now. (Will be fixed in a dedicated commit.)
> - insert missing ;
> 
> Signed-off-by: Christian Brauner <christian.brau...@mailbox.org>
> ---
>  src/lxc/lxc_ls.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/src/lxc/lxc_ls.c b/src/lxc/lxc_ls.c
> index bfe37cb..b270ff1 100644
> --- a/src/lxc/lxc_ls.c
> +++ b/src/lxc/lxc_ls.c
> @@ -93,8 +93,8 @@ static void ls_free_arr(char **arr, size_t size);
>   * container) cannot be retrieved.
>   */
>  static int ls_get(struct ls **m, size_t *size, const struct lxc_arguments 
> *args,
> -             struct lengths *lht, const char *basepath, const char *parent,
> -             unsigned int lvl, char **lockpath, size_t len_lockpath);
> +             const char *basepath, const char *parent, unsigned int lvl,
> +             char **lockpath, size_t len_lockpath);
>  static char *ls_get_cgroup_item(struct lxc_container *c, const char *item);
>  static char *ls_get_config_item(struct lxc_container *c, const char *item,
>               bool running);
> @@ -102,7 +102,6 @@ static char *ls_get_groups(struct lxc_container *c, bool 
> running);
>  static char *ls_get_ips(struct lxc_container *c, const char *inet);
>  struct wrapargs {
>       const struct lxc_arguments *args;
> -     struct lengths *lht;
>       int pipefd[2];
>       size_t *size;
>       const char *parent;
> @@ -233,7 +232,7 @@ int main(int argc, char *argv[])
>       /* &(char *){NULL} is no magic. It's just a compound literal which
>        * avoids having a pointless variable in main() that serves no purpose
>        * here. */
> -     int status = ls_get(&ls_arr, &ls_size, &my_args, &max_len, "", NULL, 0, 
> &(char *){NULL}, 0);
> +     int status = ls_get(&ls_arr, &ls_size, &my_args, "", NULL, 0, &(char 
> *){NULL}, 0);
>       if (!ls_arr && status == 0)
>               /* We did not fail. There was just nothing to do. */
>               exit(EXIT_SUCCESS);
> @@ -307,8 +306,8 @@ static void ls_free_arr(char **arr, size_t size)
>  }
>  
>  static int ls_get(struct ls **m, size_t *size, const struct lxc_arguments 
> *args,
> -             struct lengths *lht, const char *basepath, const char *parent,
> -             unsigned int lvl, char **lockpath, size_t len_lockpath)
> +             const char *basepath, const char *parent, unsigned int lvl,
> +             char **lockpath, size_t len_lockpath)
>  {
>       /* As ls_get() is non-tail recursive we face the inherent danger of
>        * blowing up the stack at some level of nesting. To have at least some
> @@ -476,7 +475,6 @@ static int ls_get(struct ls **m, size_t *size, const 
> struct lxc_arguments *args,
>                       /* Send in the parent for the next nesting level. */
>                       wargs.parent = l->name;
>                       wargs.args = args;
> -                     wargs.lht = lht;
>  
>                       pid_t out;
>  
> @@ -528,11 +526,11 @@ static int ls_get(struct ls **m, size_t *size, const 
> struct lxc_arguments *args,
>                       if (ls_remove_lock(path, name, lockpath, &len_lockpath, 
> true) == -1)
>                               goto put_and_next;
>  
> -                     ls_get(m, size, args, lht, newpath, l->name, lvl + 1, 
> lockpath, len_lockpath);
> +                     ls_get(m, size, args, newpath, l->name, lvl + 1, 
> lockpath, len_lockpath);
>                       free(newpath);
>  
>                       /* Remove the lock. No need to check for failure here. 
> */
> -                     ls_remove_lock(path, name, lockpath, &len_lockpath, 
> false)
> +                     ls_remove_lock(path, name, lockpath, &len_lockpath, 
> false);
>               }
>  
>  put_and_next:
> @@ -688,13 +686,16 @@ static bool ls_has_all_grps(const char *has, const char 
> *must)
>       if (tmp_must_len > tmp_has_len)
>               tmp_must_len = tmp_has_len = 0;
>  
> -     bool broke_out = false;
> +     bool broke_out = false, ran_once = false;
>       char **s, **t;
>       /* Check if container has all relevant groups. */
>       for (s = tmp_must; (tmp_must_len > 0) && (tmp_has_len > 0) && s && *s; 
> s++) {
>               if (broke_out)
>                       broke_out = false;

Can you explain in what cases broke_out could be true here?

broke_out is not 'static' and starts out false, and you break out and return
from the fn any time you set broke_out to true.

(My question pertains to the original code;  I should've noticed it last
time)

> +             else if (!broke_out && ran_once)
> +                     break;
>               for (t = tmp_has; t && *t; t++) {
> +                     ran_once = true;
>                       if (strcmp(*s, *t) == 0) {
>                               broke_out = true;
>                               break;
> @@ -946,7 +947,7 @@ static int ls_get_wrapper(void *wrap)
>       /* &(char *){NULL} is no magic. It's just a compound literal which
>        * avoids having a pointless variable in main() that serves no purpose
>        * here. */
> -     ls_get(&m, &len, wargs->args, wargs->lht, "", wargs->parent, 
> wargs->nestlvl, &(char *){NULL}, 0);
> +     ls_get(&m, &len, wargs->args, "", wargs->parent, wargs->nestlvl, &(char 
> *){NULL}, 0);
>       if (!m)
>               goto out;
>  
> -- 
> 2.7.0
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel@lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to