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?
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.
"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

Reply via email to