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

Re: [PATCH v3 1/2] [GSoC] dir_iterator: iterate over dir after its contents

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Mar 26, 2017, 22:40 UTC
Message-ID
<c9b6276a-331f-f42e-94b4-bb38109485a4@alum.mit.edu>
In-Reply-To
<1490465551-71056-2-git-send-email-bnmvco@gmail.com>
On 03/25/2017 07:12 PM, Daniel Ferreira wrote:
Show 19 quoted lines
> Create an option for the dir_iterator API to iterate over a directory
> path only after having iterated through its contents. This feature was
> predicted, although not implemented by 0fe5043 ("dir_iterator: new API
> for iterating over a directory tree", 2016-06-18).
> 
> This is useful for recursively removing a directory and calling rmdir()
> on a directory only after all of its contents have been wiped.
> 
> An "options" member has been added to the dir_iterator struct. It
> contains the "iterate_dirs_after_files" flag, that enables the feature
> when set to 1. Default behavior continues to be iterating over directory
> paths before its contents.
> 
> Two inline functions have been added to dir_iterator's code to avoid
> code repetition inside dir_iterator_advance() and make code more clear.
> 
> No particular functions or wrappers for setting the options struct's
> fields have been added to avoid either breaking the current dir_iterator
> API or over-engineering an extremely simple option architecture.

This patch would be easier to read if it were split into two: one extracting the new functions and changing old code to use them, and a second adding the new functionality. As one patch, is is hard to see quickly which changes have what purpose.

I also suggest adding a new `unsigned int flags` parameter to `dir_iterator_begin`. I think that's more natural, because it doesn't make sense to change the iteration order during an iteration. It's not much of a problem to change the API given that all callers are in the same codebase. If you were to forget to convert any callers (or if a different in-flight patch series were to add a new caller using the old call style), the compiler would complain, and the problem would be obvious and easy to fix.

I didn't actually read the patch carefully yet because I don't have time this evening to seek out the interesting parts in the long diff.

Michael
> [...]
Previous: Daniel FerreiraNext: Michael Haggerty
Message 4 of 6 in “[GSoC] remove_subtree(): reimplement using iterators”
  1. 0/2 [GSoC] remove_subtree(): reimplement using iteratorsDaniel Ferreira, Mar 25, 2017
  2. 2/2 [GSoC] remove_subtree(): reimplement using iteratorsDaniel Ferreira, Mar 25, 2017
  3. 1/2 [GSoC] dir_iterator: iterate over dir after its contentsDaniel Ferreira, Mar 25, 2017
  4. Michael HaggertyMar 26, 2017
  5. Michael HaggertyMar 26, 2017
  6. Stefan BellerMar 28, 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.