Re: [GSoC][PATCH 1/2] clone: extract function from copy_or_link_directory

From: Christian Couder <christian.couder@gmail.com>
Date: Feb 16, 2019, 06:23 UTC
Message-ID: <CAP8UFD1pFeV6wUPpqNHkYB_3m=HQYFssJzTSrhfsyg0p+svDgw@mail.gmail.com>
In-Reply-To: <20190215154913.18800-2-matheus.bernardino@usp.br>

On Fri, Feb 15, 2019 at 5:39 PM Matheus Tavares <matheus.bernardino@usp.br> wrote:

Show 5 quoted lines

>
> 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 patch
> "clone: use dir-iterator to avoid explicit dir traversal".

"which will be done in a following patch" is enough and perhaps even better as the following patch can then be changed independently of this one.

> Also makes
> code more readable.
>
> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>

[...]

Show 6 quoted lines

> +static void mkdir_if_missing(const char *pathname, mode_t mode)
> +{
> +       /*
> +        * Create a dir at pathname unless there's already one.
> +        */
> +       struct stat buf;

I know that the variable was already called "buf" in copy_or_link_directory() and that there are a few other places in the code where a 'struct stat' is called "buf", but in most places the 'struct stat' variables are called "st":

$ git grep 'struct stat ' '*.c' | perl -ne 'print "$1\n" if (m/struct
stat ([\w_]+)/);' | sort | uniq -c | sort -nr
    129 st
      6 sb
      3 buf
      2 statbuf
      1 st_stdin
      1 st_git
      1 statbuffer
      1 st2
      1 st1
      1 nst
      1 loginfo
      1 cwd_stat
      1 argstat

So I wonder if we should use "st" here instead of "buf". We also often use "buf" for 'struct strbuf' variables which can be confusing.

Otherwise the patch looks good to me.