Quoting Richard Weinberger (rich...@nod.at):
> Am 17.04.2013 18:00, schrieb Serge Hallyn:
> >Quoting Richard Weinberger (rich...@nod.at):
> >>Reimplement mkdir_p() such that it:
> >> ...handles relativ paths correctly. (currently it crashes)
> >> ...does not rely on dirname().
> >> ...is not recursive.
> >> ...is shorter. ;-)
> >
> >Looks good, thanks. Yeah I prefer non-recursive. Three
> >comments though,
> >
> >>Signed-off-by: Richard Weinberger <rich...@nod.at>
> >>---
> >> src/lxc/utils.c | 48 +++++++++++++++++-------------------------------
> >> 1 file changed, 17 insertions(+), 31 deletions(-)
> >>
> >>diff --git a/src/lxc/utils.c b/src/lxc/utils.c
> >>index e07ca7b..9794553 100644
> >>--- a/src/lxc/utils.c
> >>+++ b/src/lxc/utils.c
> >>@@ -95,39 +95,25 @@ extern int get_u16(unsigned short *val, const char
> >>*arg, int base)
> >> return 0;
> >> }
> >>
> >>-static int is_all_slashes(char *path)
> >>-{
> >>- while (*path && *path == '/')
> >>- path++;
> >>- if (*path)
> >>- return 0;
> >>- return 1;
> >>-}
> >>-
> >> extern int mkdir_p(char *dir, mode_t mode)
> >> {
> >>- int ret;
> >>- char *d;
> >>-
> >>- if (is_all_slashes(dir))
> >>- return 0;
> >>-
> >>- d = strdup(dir);
> >>- if (!d)
> >>- return -1;
> >>-
> >>- ret = mkdir_p(dirname(d), mode);
> >>- free(d);
> >>- if (ret)
> >>- return -1;
> >>-
> >>- if (!access(dir, F_OK))
> >>- return 0;
> >>-
> >>- if (mkdir(dir, mode)) {
> >>- SYSERROR("failed to create directory '%s'\n", dir);
> >>- return -1;
> >>- }
> >>+ char *tmp = dir;
> >>+ char *orig = dir;
> >>+ char *makeme;
> >>+
> >>+ do {
> >>+ dir = tmp + strspn(tmp, "/");
> >>+ tmp = dir + strcspn(dir, "/");
> >>+ makeme = strndupa(orig, dir - orig);
> >
> >strndupa *can* fail and return NULL.
>
> How?
I was hoping that smart libc on embedded would return NULL if it sees
there isn't enough space. I misread the strndupa manpage to say it
returns NULL on failure - I see there is no such promise.
> strndupa() uses alloca(), which allocates memory on the stack.
> *If* it fails we have already overrun the stack and are on a one-way trip
> to lala land. :-)
>
> >
> >>+ if (*makeme) {
> >>+ if (!access(makeme, F_OK))
> >>+ return 0;
> >
> >did you mean to continue here?
>
> No, if the access() we have to stop.
=========
If the access() does what? access("/var", F_OK) should immediately
return 0, so you're saying mkdir_p("/var/lib/lxc/c1") should immediately
fail? It's entirely possible I'm not thinking right, but...
> "mkdir -p /foo/bar" also just exists if it is not allowed to access one
> parent directory.
>
> >>+ if (mkdir(makeme, mode)) {
> >
> >As people are starting to want to thread, I think it's worth making sure
> >all mkdirs and creats check for errno=-EEXIST even if they've just
> >checked with access. Certainly with the cgroup setup a race is possible
> >when starting two simultaneous containers first time after boot using
> >the api.
>
> Yes, you are right. I'll update my patch to take care of this.
>
> Thanks,
> //richard
------------------------------------------------------------------------------
Precog is a next-generation analytics platform capable of advanced
analytics on semi-structured data. The platform includes APIs for building
apps and a phenomenal toolset for data science. Developers can use
our toolset for easy data analysis & visualization. Get a free account!
http://www2.precog.com/precogplatform/slashdotnewsletter
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel
