On Tue, Jan 19, 2016 at 09:02:00PM +0000, Serge Hallyn wrote:
> 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)
This is a simple subset problem. We want to determine whether tmp_must is a
subset of tmp_has. For each string in tmp_must we check in the inner loop if it
is contained withing tmp_has. If we find it we set brok_out to true and break
out of the inner loop and advance to the next string in tmp_must in the outer
loop and reset broke_out to false. This is a pretty dumb algorithm (O(m*n))
which when I jotted it down missed the boolean ran_once (and why I want to
change it later). ran_once tells us that the loop ran once and a failure to find
one of the string of tmp_must in tmp_has means we can stop.
>
> > + 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
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
