On Sat, Feb 16, 2019 at 4:23 AM Christian Couder
<christian.couder@xxxxxxxxx> wrote:
>
> On Fri, Feb 15, 2019 at 5:39 PM Matheus Tavares
> <matheus.bernardino@xxxxxx> 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 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.
>
Ok! I will fix that in v2.
> > Also makes
> > code more readable.
> >
> > Signed-off-by: Matheus Tavares <matheus.bernardino@xxxxxx>
>
> [...]
>
> > +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.
>
Right! I'm changing that in v2 too.
Thanks,
Matheus
> Otherwise the patch looks good to me.
