{"thread":{"id":"7651","subject":"[PATCH] create_directories: simplify and avoid repeated copying","startedAt":"2007-04-12T20:29:10Z","lastAt":"2007-04-14T14:11:14Z","messageCount":2,"participants":["Geert Bosch","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"39319","messageId":"20070414013003.B0B4C2F1DC3@geert-boschs-computer.local","threadId":"7651","inReplyTo":null,"subject":"[PATCH] create_directories: simplify and avoid repeated copying","fromName":"Geert Bosch","fromEmail":"bosch@gnat.com","sentAt":"2007-04-12T20:29:10Z","receivedAt":"2007-04-12T20:29:10Z","isPatch":true,"sender":{"key":"bosch@gnat.com","avatar":null},"body":"Signed-off-by: Geert Bosch <bosch@gnat.com>\n---\nI needed to change the logic of this function a bit for\ndoing some local hacks^Wmodifications and thought it might\nbe a slight improvement/simplification.\nNo change in functionality and passed 'make test'.\n\n entry.c |   23 +++++++++--------------\n 1 files changed, 9 insertions(+), 14 deletions(-)\n\ndiff --git a/entry.c b/entry.c\nindex d72f811..f966c59 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -3,24 +3,19 @@\n \n static void create_directories(const char *path, struct checkout *state)\n {\n-\tint len = strlen(path);\n-\tchar *buf = xmalloc(len + 1);\n-\tconst char *slash = path;\n+\tchar *buf = xstrdup(path);\n+\tchar *slash = buf;\n \n \twhile ((slash = strchr(slash+1, '/')) != NULL) {\n-\t\tlen = slash - path;\n-\t\tmemcpy(buf, path, len);\n-\t\tbuf[len] = 0;\n+\t\t*slash = 0;\n \t\tif (mkdir(buf, 0777)) {\n-\t\t\tif (errno == EEXIST) {\n-\t\t\t\tstruct stat st;\n-\t\t\t\tif (len > state->base_dir_len && state->force && !unlink(buf) && !mkdir(buf, 0777))\n-\t\t\t\t\tcontinue;\n-\t\t\t\tif (!stat(buf, &st) && S_ISDIR(st.st_mode))\n-\t\t\t\t\tcontinue; /* ok */\n-\t\t\t}\n-\t\t\tdie(\"cannot create directory at %s\", buf);\n+\t\t\tstruct stat st;\n+\t\t\tint force = (slash - buf) > state->base_dir_len && state->force;\n+\t\t\tif (errno != EEXIST || ((!force || unlink(buf) || mkdir(buf, 0777)) &&\n+\t\t\t    (stat(buf, &st) || !S_ISDIR(st.st_mode))))\n+\t\t\t\tdie(\"cannot create directory at %s\", buf);\n \t\t}\n+\t\t*slash='/';\n \t}\n \tfree(buf);\n }\n-- \n1.5.1\n"},{"id":"39337","messageId":"7vlkgv9hbx.fsf@assigned-by-dhcp.cox.net","threadId":"7651","inReplyTo":"20070414013003.B0B4C2F1DC3@geert-boschs-computer.local","subject":"Re: [PATCH] create_directories: simplify and avoid repeated copying","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-14T14:11:14Z","receivedAt":"2007-04-14T14:11:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Geert Bosch <bosch@gnat.com> writes:\n\n> Signed-off-by: Geert Bosch <bosch@gnat.com>\n> ---\n> I needed to change the logic of this function a bit for\n> doing some local hacks^Wmodifications and thought it might\n> be a slight improvement/simplification.\n> No change in functionality and passed 'make test'.\n\nI like the removal of repeated copying, but I wonder if the\ndenser condition is really easier to read.\n\nI tried to like it, but I needed to reindent it first, and then\nI needed to read it three times to convince myself that they are\nequivalent.\n\n        if (errno != EEXIST\n            || ((!force || unlink(buf) || mkdir(buf, 0777))\n                && (stat(buf, &st) || !S_ISDIR(st.st_mode))) )\n                die(\"cannot create directory at %s\", buf);\n\nIn the end, I managed to convince myself that this denser\nversion is doing the same thing as the original, and it is nicer\nto see it in only three lines, I am not sure if it is easier to\nread anymore, judging from the time it took me to reach that\nconclusion...\n"}]}