On Tue, Feb 26, 2019 at 12:18 PM Matheus Tavares
<[email protected]> wrote:
>
> 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 <[email protected]>
> ---
> 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.
> + 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
