git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] [GSoC] remove_subtree(): reimplement using iterators

From
Duy Nguyen <pclouds@gmail.com>
Date
Mar 25, 2017, 11:51 UTC
Message-ID
<CACsJy8Aajf1Lb4mKC=NEUQTNy7QRK99Aont_UCRnHJfho8n+UA@mail.gmail.com>
In-Reply-To
<1490441257-44727-1-git-send-email-bnmvco@gmail.com>
On Sat, Mar 25, 2017 at 6:27 PM, Daniel Ferreira <bnmvco@gmail.com> wrote:
Show 77 quoted lines
> Use dir_iterator to traverse through remove_subtree()'s directory tree,
> avoiding the need for recursive calls to readdir(). Simplify
> remove_subtree()'s code.
>
> A conversion similar in purpose was previously done at 46d092a
> ("for_each_reflog(): reimplement using iterators", 2016-05-21).
>
> Signed-off-by: Daniel Ferreira <bnmvco@gmail.com>
> ---
>
> This is a second-version patch of the Google Summer of Code microproject for
> refactoring recursive readdir() calls to use dir_iterator instead. v1 can be
> found in:
>
> https://public-inbox.org/git/CAGZ79kZwT-9mHTiOJ5CEjk2wDFkn6+NcogjX0=vjhsAh16ANYg@mail.gmail.com/T/#mae023e7a7d7626f00e0923833c4359f5af493730
>
> Additionally, for debugging purposes I turned remove_subtree() into a no-op
> and ran git tests. Some failures were at:
>
> * t2000-checkout-cache-clash.sh
> * t2003-checkout-cache-mkdir.sh
>
> If you guys could check those files out and warn me if any additional tests
> would be welcome, please let me know.
>
> Thanks.
>
>  entry.c | 28 +++++++---------------------
>  1 file changed, 7 insertions(+), 21 deletions(-)
>
> diff --git a/entry.c b/entry.c
> index c6eea240b..3cb92592d 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -2,6 +2,8 @@
>  #include "blob.h"
>  #include "dir.h"
>  #include "streaming.h"
> +#include "iterator.h"
> +#include "dir-iterator.h"
>
>  static void create_directories(const char *path, int path_len,
>                                const struct checkout *state)
> @@ -46,29 +48,13 @@ static void create_directories(const char *path, int path_len,
>
>  static void remove_subtree(struct strbuf *path)
>  {
> -       DIR *dir = opendir(path->buf);
> -       struct dirent *de;
> -       int origlen = path->len;
> -
> -       if (!dir)
> -               die_errno("cannot opendir '%s'", path->buf);
> -       while ((de = readdir(dir)) != NULL) {
> -               struct stat st;
> -
> -               if (is_dot_or_dotdot(de->d_name))
> -                       continue;
> -
> -               strbuf_addch(path, '/');
> -               strbuf_addstr(path, de->d_name);
> -               if (lstat(path->buf, &st))
> -                       die_errno("cannot lstat '%s'", path->buf);
> -               if (S_ISDIR(st.st_mode))
> -                       remove_subtree(path);
> -               else if (unlink(path->buf))
> +       struct dir_iterator *diter = dir_iterator_begin(path->buf);
> +
> +       while (dir_iterator_advance(diter) == ITER_OK) {
> +               if (unlink(diter->path.buf))
>                         die_errno("cannot unlink '%s'", path->buf);
> -               strbuf_setlen(path, origlen);
>         }
> -       closedir(dir);
> +
>         if (rmdir(path->buf))
>                 die_errno("cannot rmdir '%s'", path->buf);

Even though it's very nice that lots of code is deleted. This is not entirely correct, is it? Before this patch, rmdir() is called for every recursive remove_subtree() call. After this patch, it's only called once (and likely fails unless you have no subdirectories).

>  }
> --
> 2.12.1.433.g82305b74f.dirty
>
-- 
Duy
Previous: Daniel FerreiraNext: Daniel Ferreira (theiostream)
Message 2 of 4 in “[GSoC] remove_subtree(): reimplement using iterators”
  1. [GSoC] remove_subtree(): reimplement using iteratorsDaniel Ferreira, Mar 25, 2017
  2. Duy NguyenMar 25, 2017
  3. Daniel Ferreira (theiostream)Mar 25, 2017
  4. Duy NguyenMar 25, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.