Daniel Ferreira <bnmvco@xxxxxxxxx> writes:
> Create helpers to dir_iterator_advance(). Make dir_iterator_advance()'s
> code more legible and allow some behavior to be reusable.
>
> Signed-off-by: Daniel Ferreira <bnmvco@xxxxxxxxx>
> ---
This is the kind of change we typically call "refactoring" ---
factoring out reusable helpers out of existing code, without
changing the behaviour of the code at all.
> +static int set_iterator_data(struct dir_iterator_int *iter, struct dir_iterator_level *level)
> +{
If you are going to fold line in a patch that immediately follows
this one, do so from the get-go here.
I am not happy with the name of the function in that "set data" does
not convey as much information as it spends words.
Conceptually, this is called once per each new directory entry
readdir() returns, and the most important "iterator data" that is
set to iter structure per loop is to update iter->base.path with
de->d_name; everything else is secondary and derived data from that.
And from that point of view, I have trouble with this function not
getting "de" passed to it, but the most important setting is instead
done by its caller.
If the function were called "adjust_iterator_data()", I wouldn't
find this function so troubling, I guess; after the caller sets the
iter.base.path, all the secondary data that can be derived from it
that are used by the caller of "advance" are set by this function.
> + if (lstat(iter->base.path.buf, &iter->base.st) < 0) {
> + if (errno != ENOENT)
> + warning("error reading path '%s': %s",
> + iter->base.path.buf,
> + strerror(errno));
> + return -1;
> + }
> +
> + /*
> + * We have to set these each time because
> + * the path strbuf might have been realloc()ed.
> + */
> + iter->base.relative_path =
> + iter->base.path.buf + iter->levels[0].prefix_len;
> + iter->base.basename =
> + iter->base.path.buf + level->prefix_len;
> + level->dir_state = DIR_STATE_ITER;
> +
> + return 0;
> +}
> +