Replace usage of opendir/readdir/closedir API to traverse directories
recursively, at copy_or_link_directory function, by the dir-iterator
API. This simplifies the code and avoid recursive calls to
copy_or_link_directory.

This process also brings some safe behaviour changes to
copy_or_link_directory:
 - It will no longer follows symbolic links. This is not a problem,
   since the function is only used to copy .git/objects directory, and
   symbolic links are not expected there.
 - Hidden directories won't be skipped anymore. In fact, it is odd that
   the function currently skip hidden directories but not hidden files.
   The reason for that could be unintentional: probably the intention
   was to skip '.' and '..' only, but it ended up accidentally skipping
   all directories starting with '.'. Again, it must not be a problem
   not to skip hidden dirs since hidden dirs/files are not expected at
   .git/objects.
 - Now, copy_or_link_directory will call die() in case of an error on
   openddir, readdir or lstat, inside dir_iterator_advance. That means
   it will abort in case of an error trying to fetch any iteration
   entry.

Signed-off-by: Matheus Tavares <matheus.bernard...@usp.br>
---
Changes in v2:
 - Improved patch message
 - Removed a now unused variable
 - Put warning on stat error back
 - Added pedantic option to dir-iterator initialization
 - Modified copy_or_link_directory not to skip hidden paths

 builtin/clone.c | 47 ++++++++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 862d2ea69c..515dc91d63 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -23,6 +23,8 @@
 #include "transport.h"
 #include "strbuf.h"
 #include "dir.h"
+#include "dir-iterator.h"
+#include "iterator.h"
 #include "sigchain.h"
 #include "branch.h"
 #include "remote.h"
@@ -411,42 +413,45 @@ static void mkdir_if_missing(const char *pathname, mode_t 
mode)
 }
 
 static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
-                                  const char *src_repo, int src_baselen)
+                                  const char *src_repo)
 {
-       struct dirent *de;
-       struct stat buf;
        int src_len, dest_len;
-       DIR *dir;
-
-       dir = opendir(src->buf);
-       if (!dir)
-               die_errno(_("failed to open '%s'"), src->buf);
+       struct dir_iterator *iter;
+       int iter_status;
+       struct stat st;
 
        mkdir_if_missing(dest->buf, 0777);
 
+       iter = dir_iterator_begin(src->buf, 1);
+
        strbuf_addch(src, '/');
        src_len = src->len;
        strbuf_addch(dest, '/');
        dest_len = dest->len;
 
-       while ((de = readdir(dir)) != NULL) {
+       while ((iter_status = dir_iterator_advance(iter)) == ITER_OK) {
                strbuf_setlen(src, src_len);
-               strbuf_addstr(src, de->d_name);
+               strbuf_addstr(src, iter->relative_path);
                strbuf_setlen(dest, dest_len);
-               strbuf_addstr(dest, de->d_name);
-               if (stat(src->buf, &buf)) {
+               strbuf_addstr(dest, iter->relative_path);
+
+               /*
+                * dir_iterator_advance already calls lstat to populate iter->st
+                * but, unlike stat, lstat does not checks for permissions on
+                * the given path.
+                */
+               if (stat(src->buf, &st)) {
                        warning (_("failed to stat %s\n"), src->buf);
                        continue;
                }
-               if (S_ISDIR(buf.st_mode)) {
-                       if (de->d_name[0] != '.')
-                               copy_or_link_directory(src, dest,
-                                                      src_repo, src_baselen);
+
+               if (S_ISDIR(iter->st.st_mode)) {
+                       mkdir_if_missing(dest->buf, 0777);
                        continue;
                }
 
                /* Files that cannot be copied bit-for-bit... */
-               if (!strcmp(src->buf + src_baselen, "/info/alternates")) {
+               if (!strcmp(iter->relative_path, "info/alternates")) {
                        copy_alternates(src, dest, src_repo);
                        continue;
                }
@@ -463,7 +468,11 @@ static void copy_or_link_directory(struct strbuf *src, 
struct strbuf *dest,
                if (copy_file_with_time(dest->buf, src->buf, 0666))
                        die_errno(_("failed to copy file to '%s'"), dest->buf);
        }
-       closedir(dir);
+
+       if (iter_status != ITER_DONE) {
+               strbuf_setlen(src, src_len);
+               die(_("failed to iterate over '%s'"), src->buf);
+       }
 }
 
 static void clone_local(const char *src_repo, const char *dest_repo)
@@ -481,7 +490,7 @@ static void clone_local(const char *src_repo, const char 
*dest_repo)
                get_common_dir(&dest, dest_repo);
                strbuf_addstr(&src, "/objects");
                strbuf_addstr(&dest, "/objects");
-               copy_or_link_directory(&src, &dest, src_repo, src.len);
+               copy_or_link_directory(&src, &dest, src_repo);
                strbuf_release(&src);
                strbuf_release(&dest);
        }
-- 
2.20.1

Reply via email to