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

Re: [PATCH] entry.c: use dir-iterator to avoid explicit dir traversal

From
Junio C Hamano <gitster@pobox.com>
Date
Dec 9, 2019, 21:18 UTC
Message-ID
<xmqqv9qptcyz.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<CAHd-oW4MzH9Wrb2zNPF0JBN8+FRB4NtDX0166Qt2Kbqs0mDgcw@mail.gmail.com>
Matheus Tavares Bernardino <matheus.bernardino@usp.br> writes:
Show 25 quoted lines
>> The changes pass the test suite t/ and Travis CI.
>> Please point out any mistakes.
>> ...
>> -               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))
>> -                       die_errno("cannot unlink '%s'", path->buf);
>> -               strbuf_setlen(path, origlen);
>> +               if (unlink(iter->path.buf)) {
>
> unlink()-ing a directory in Linux will return a EISDIR error. So I
> think you still need to use S_ISDIR() to check if iter->path.buf is a
> directory and call rmdir(), in this case.
>
> However, note that the dir-iterator API gives entries in pre-order.
> I.e. a directory appears before its subentries. In the use case of
> remove_subtree(), though, we need to traverse in post-order, since we
> have to remove the subentries before removing the directory where they
> reside. My suggestion is that you add a preliminary patch,
> implementing a new DIR_ITERATOR_POST_ORDER flag to dir-iterator.h, and
> then use it in this patch.

Thanks for a review and a few hints to nudge a new contributor in the right direction. Very much appreciated.

I wonder why the bugs in this patch weren't caught by self test we already have, by the way. We need a bit better test coverage, perhaps?

Previous: Matheus Tavares BernardinoNext: Matheus Tavares Bernardino
Message 4 of 6 in “entry.c: use dir-iterator to avoid explicit dir traversal”
  1. entry.c: use dir-iterator to avoid explicit dir traversalotalpster@gmail.com, Dec 8, 2019
  2. Derrick StoleeDec 9, 2019
  3. Matheus Tavares BernardinoDec 9, 2019
  4. Junio C HamanoDec 9, 2019
  5. Matheus Tavares BernardinoDec 10, 2019
  6. otalpsterDec 16, 2019

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.