[PATCH v6] erofs-utils: lib: add API to iterate dirs in EROFS

[Gao Xiang]

On Wed, Dec 15, 2021 at 09:53:19AM -0800, Kelvin Zhang wrote:
> I don't like the current API design.
> 

Ok, if you don't like the current way, I will convert the author
instead.

Here is my answer why it designs like this:
 
> 1. Input and output to erofs_iterate_dir are mixed in the same struct. It's
> unclear to the caller which members are required input, and which ones will
> be filled out by erofs_iterate_dir. I prefer the old way where input
> parameters are explicitly listed, and the context struct consists of only
> output members.

The problem is there are too many input paramters, and I don't think
they needs to be passed on stack (maybe some by registers, but it
depends) over and over again.

Actually if you looked at some kernel code, there are many such usage:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/pagemap.h#n982
struct readahead_control;

And
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/fs.h#n2023
dir_context, etc.

> 2. When erofs_readdir_cb is called, is the callback allowed to modify
> members inside erofs_dir_context ? If not, the pointer should be marked as
> const, or pass the struct by value. Or explicitly document that any
> modification to the context struct will be ignored.

Ok, actually I'm either fine to add or not add `const'. But I have to
say, erofs-utils is not a C++ project.

But yes, marking const is a good habit. I'm fine to take all of this.

> 3. We are doing dynamic memory allocations inside a loop. Can we just use
> two stack buffers?

I thought before, but I do also care stack overflow (it somewhat happens
on my Mac OS computer.) Dynamic-sized object on stack is tricky, I'd
like to avoid this like what kernel does.

> 4. Many of the current use cases(dump, fsck, android) want to traverse
> directories recursively. Instead of duplicating the work and let each user
> write its own logic to do recursive traversal, provide a sensical default
> implementation that traverses directory recursively? This should be a
> straightforward wrapper around erofs_iterate_dir. For example, adding
> a erofs_iterate_dir_recursive API.

Ok, if you think that saves code, that's fine.

However, fuse don't need to recursively traverse directories. And many
cases don't need to traverse directories in the DFS approach. So, a new
erofs_iterate_dir_recursive API is ok if it really saves code.

Thanks,
Gao Xiang