Re: New version of function mkdirhier()
On Sun, 2012-01-15 at 02:22 +0100, Samuel Thibault wrote:
> Svante Signell, le Sun 15 Jan 2012 01:23:02 +0100, a écrit :
...
> > What is personal criticism then, and what is the alternative,
> > non-personal?
>
> Comments on code is non-personal for sure.
>
> > Getting feedback on a proposed patch, isn't that personal?
>
> No.
Never mind: Attached is a dynamic-allocation version of mkdirhier.c
together with test cases. For reference the original code,
mkdirhier_orig.c, and my version, mkdirhier_new.c are attached with
test cases for these versions too. Is it possible to simplify the code
somewhat, especially for the first version? That one will be part of the
patch for libtar (almost complete).
Thanks!
SOURCES = test_mkdirhier.c mkdirhier.c mkdirhier_orig.c mkdirhier_new.c
OBJECTS=$(SOURCES:.c=.o)
EXECUTABLE=test_mkdirhier
CC = gcc
CFLAGS = -g -c -Wall -Werror
LDFLAGS = -g
all: $(SOURCES) $(EXECUTABLE)
$(EXECUTABLE): $(OBJECTS)
$(CC) $(LDFLAGS) $(OBJECTS) -o $@ -lbsd
.c.o:
$(CC) $(CFLAGS) $< -o $@
src_clean:
rm *.o *~
data_clean:
\rm -r .da da ../da /tmp/test
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
int mkdirhier(char *);
int mkdirhier_orig(char *);
int mkdirhier_new(char *);
int main(void)
{
char *path[33]={".", "./", ".//", "..", "../", "..//", "/tmpp",
"/tmp", "/tmp/test", "//tmp/test", "/tmp/test//",
"da", "da/", "./da", "./da/", ".//da", "./da//",
".da", ".da/", "./.da", ".da/", "././/da", "./../da//",
"da/b", "da/b/", "da/b//", "./da", "../da", "../da/",
"da/b/c", "da/b/c//", "da//b//c/ddee/f/", "da/b/c/ddee/f"};
int result = 0, i;
printf("mkdirhier:\n======================\n");
for (i=0; i<33; i++) {
printf ("%i) path = %s\n", i, path[i]);
result = mkdirhier(path[i]);
if (result == 1)
printf ("all directories exist\n");
else if (result == 0)
printf ("all directories created\n");
else
printf ("error: %s\n", strerror(errno));
printf("======================\n");
}
printf("\n\nmkdirhier_orig:\n======================\n");
for (i=0; i<33; i++) {
printf ("%i) path = %s\n", i, path[i]);
result = mkdirhier_orig(path[i]);
if (result == 1)
printf ("all directories exist\n");
else if (result == 0)
printf ("all directories created\n");
else
printf ("error: %s\n", strerror(errno));
printf("======================\n");
}
printf("\n\nmkdirhier_new:\n======================\n");
for (i=0; i<33; i++) {
printf ("%i) path = %s\n", i, path[i]);
result = mkdirhier_new(path[i]);
if (result == 1)
printf ("all directories exist\n");
else if (result == 0)
printf ("all directories created\n");
else
printf ("error: %s\n", strerror(errno));
printf("======================\n");
}
return 0;
}
#include <bsd/string.h>
#include <string.h>
#include <stdio.h>
#include <errno.h>
#include <stdlib.h>
#include <sys/stat.h>
/*
** mkdirhier() - create all directories in a given path
** returns:
** 0 success
** 1 all directories already exist
** -1 (and sets errno) error
*/
int
mkdirhier(char *path)
{
char *src = NULL, *srcp = NULL, *dst = NULL;
char *dirp, *nextp = NULL;
int retval = 1, len;
len = strlen(path);
if ((src = strdup(path)) == NULL)
{
errno = ENOMEM;
return -1;
}
srcp = src;
nextp = src;
if ((dst = malloc(len + 1)) == NULL)
{
errno = ENOMEM;
return -1;
}
dst[0] = '\0';
if (path[0] == '/')
strcpy(dst, "/");
while ((dirp = strsep(&nextp, "/")) != NULL)
{
if (*dirp == '\0')
continue;
if (dst[0] != '\0')
strcat(dst, "/");
strcat(dst, dirp);
printf("Creating directory dst = %s\n",dst);
if (mkdir(dst, 0777) == -1)
{
if (errno != EEXIST)
{
free(srcp);
free(dst);
return -1;
}
}
else
retval = 0;
}
free(srcp);
free(dst);
return retval;
}
#include <bsd/string.h>
#include <stdio.h>
#include <errno.h>
#include <stdlib.h>
#include <sys/stat.h>
/* MAXPATHLEN in Linux */
#include <sys/param.h>
/*
** mkdirhier() - create all directories in a given path
** returns:
** 0 success
** 1 all directories already exist
** -1 (and sets errno) error
*/
int
mkdirhier_orig(char *path)
{
char src[MAXPATHLEN], dst[MAXPATHLEN] = "";
char *dirp, *nextp = src;
int retval = 1;
if (strlcpy(src, path, sizeof(src)) > sizeof(src))
{
errno = ENAMETOOLONG;
return -1;
}
if (path[0] == '/')
strcpy(dst, "/");
while ((dirp = strsep(&nextp, "/")) != NULL)
{
if (*dirp == '\0')
continue;
if (dst[0] != '\0')
strcat(dst, "/");
strcat(dst, dirp);
printf("Creating directory dst = %s\n",dst);
if (mkdir(dst, 0777) == -1)
{
if (errno != EEXIST)
return -1;
}
else
retval = 0;
}
return retval;
}
#include <string.h>
#include <stdio.h>
#include <errno.h>
#include <stdlib.h>
#include <sys/stat.h>
/*
** mkdirhier() - create all directories in a given path
** returns:
** 0 success
** 1 all directories already exist
** -1 (and sets errno) error
*/
int
mkdirhier_new(char *path)
{
char *src, *dst = "";
char *dirp;
int retval = 1, len = 0;
src = path;
if ((dst = malloc(strlen (src) + 1)) == NULL)
{
errno = ENOMEM;
return -1;
}
dst[0] = '\0';
/* Absolute path: Leading '/' */
if (path[0] == '/')
{
strncpy(dst, "/", 1);
src++;
}
while ((dirp = strchr(src, '/')) != NULL)
{
len = dirp - src;
if (len == 0)
{
src++;
continue;
}
if (len == 1 && src[0] == '.')
{
src++;
continue;
}
if (len == 2 && src[0] == '.' && src[1] == '.')
{
strncat(dst, src, len);
src = dirp + 1;
strncat(dst, "/", 1);
continue;
}
strncat(dst, src, len);
src = dirp + 1;
printf("Creating directory dst = %s\n",dst);
if (mkdir(dst, 0777) == -1)
{
if (errno != EEXIST)
{
free(dst);
return -1;
}
}
else
retval = 0;
strncat(dst, "/", 1);
}
/* Create dir after last '/' or no '/' in path */
len = strlen(src);
if (len == 0)
{
return retval;
}
if (len == 1 && src[0] == '.')
{
return retval;
}
if (len == 2 && src[0] == '.' && src[1] == '.')
{
return retval;
}
strncat(dst, src, len);
if (mkdir(dst, 0777) == -1)
{
if (errno != EEXIST)
{
free(dst);
return -1;
}
}
else
retval = 0;
free(dst);
return retval;
}
Reply to: