Re: [WIP RFC PATCH v2 4/5] clone: extract function from copy_or_link_directory
From: Duy Nguyen <hidden>
Date: 2019-02-26 12:18:56
On Tue, Feb 26, 2019 at 12:18 PM Matheus Tavares [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Extract dir creation code snippet from copy_or_link_directory to its own function named mkdir_if_missing. This change will help removing copy_or_link_directory's explicit recursion, which will be done in a following patch. Also makes code more readable. Signed-off-by: Matheus Tavares <redacted> --- builtin/clone.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)diff --git a/builtin/clone.c b/builtin/clone.c index cae069f03b..fd580fa98d 100644 --- a/builtin/clone.c +++ b/builtin/clone.c@@ -392,6 +392,24 @@ static void copy_alternates(struct strbuf *src, struct strbuf *dst, fclose(in); } +static void mkdir_if_missing(const char *pathname, mode_t mode) +{ + /* + * Create a dir at pathname unless there's already one.
This confused me for a second because I thought it described "st" variable. I think we usually put the description of the function on top (before the "void mkdir_if.." line). But with a such a short function and clear name like this, I don't think we need any comments.
+ */
+ struct stat st;
+
+ if (mkdir(pathname, mode)) {
Good opportunity to unindent this by doing
if (!mkdir(...
return;
but it's up to you.
quoted hunk ↗ jump to hunk
+ if (errno != EEXIST) + die_errno(_("failed to create directory '%s'"), + pathname); + else if (stat(pathname, &st)) + die_errno(_("failed to stat '%s'"), pathname); + else if (!S_ISDIR(st.st_mode)) + die(_("%s exists and is not a directory"), pathname); + } +} + static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest, const char *src_repo, int src_baselen) {@@ -404,14 +422,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest, if (!dir) die_errno(_("failed to open '%s'"), src->buf); - if (mkdir(dest->buf, 0777)) { - if (errno != EEXIST) - die_errno(_("failed to create directory '%s'"), dest->buf); - else if (stat(dest->buf, &buf)) - die_errno(_("failed to stat '%s'"), dest->buf); - else if (!S_ISDIR(buf.st_mode)) - die(_("%s exists and is not a directory"), dest->buf); - } + mkdir_if_missing(dest->buf, 0777); strbuf_addch(src, '/'); src_len = src->len; --2.20.1
-- Duy