[PATCH v2] erofs-utils: mkfs: use scandir for stable output

[Gao Xiang]

Hi Jooyung,

On 2024/12/3 08:27, Jooyung Han wrote:
> The iteration order of opendir/readdir depends on filesystem
> implementation. Hence, even with the same contents, the filesystem of
> the input directory affects the output.
> 
> In this change, opendir/readdir is replaced with scandir for stable
> order of iteration. This produces the same output regardless of the
> filesystem of the input directory.
> 
> Test: mkfs.erofs ... inputdir(ext3)
> Test: mkfs.erofs ... inputdir(tmpfs)
>    # should generate the same output
> Signed-off-by: Jooyung Han <jooyung at google.com>

Thanks for the patch.  I haven't tested the current behavior (1.8.2),
but EROFS will sort all dirents in a directory in
erofs_prepare_dir_file(). And then dump all subdirs in
erofs_mkfs_dump_tree().

It seems both dirents and inodes are sorted in the alphabetical
order, could you give more hints about this?

Thanks,
Gao Xiang

> ---
> 
> v1: https://lore.kernel.org/linux-erofs/20241202232620.3604736-1-jooyung@google.com/
> change since v1:
>   - modify commit msg (no change-id/bug/typo)
>   - rename the label to err_cleanup
> 
>   lib/inode.c | 39 ++++++++++++++-------------------------
>   1 file changed, 14 insertions(+), 25 deletions(-)
> 
> diff --git a/lib/inode.c b/lib/inode.c
> index f553bec..097deef 100644
> --- a/lib/inode.c
> +++ b/lib/inode.c
> @@ -1422,37 +1422,25 @@ static void erofs_mkfs_flushjobs(struct erofs_sb_info *sbi)
>   static int erofs_mkfs_handle_directory(struct erofs_inode *dir)
>   {
>   	struct erofs_sb_info *sbi = dir->sbi;
> -	DIR *_dir;
> -	struct dirent *dp;
> +	struct dirent *dp, **dent;
> +	int i, num_entries;
>   	struct erofs_dentry *d;
>   	unsigned int nr_subdirs, i_nlink;
>   	int ret;
>   
> -	_dir = opendir(dir->i_srcpath);
> -	if (!_dir) {
> -		erofs_err("failed to opendir at %s: %s",
> +	num_entries = scandir(dir->i_srcpath, &dent, NULL, alphasort);
> +	if (num_entries == -1) {
> +		erofs_err("failed to scandir at %s: %s",
>   			  dir->i_srcpath, erofs_strerror(-errno));
>   		return -errno;
>   	}
>   
>   	nr_subdirs = 0;
>   	i_nlink = 0;
> -	while (1) {
> +	for (i = 0; i < num_entries; free(dent[i]), i++) {
>   		char buf[PATH_MAX];
>   		struct erofs_inode *inode;
> -
> -		/*
> -		 * set errno to 0 before calling readdir() in order to
> -		 * distinguish end of stream and from an error.
> -		 */
> -		errno = 0;
> -		dp = readdir(_dir);
> -		if (!dp) {
> -			if (!errno)
> -				break;
> -			ret = -errno;
> -			goto err_closedir;
> -		}
> +		dp = dent[i];
>   
>   		if (is_dot_dotdot(dp->d_name)) {
>   			++i_nlink;
> @@ -1466,17 +1454,17 @@ static int erofs_mkfs_handle_directory(struct erofs_inode *dir)
>   		d = erofs_d_alloc(dir, dp->d_name);
>   		if (IS_ERR(d)) {
>   			ret = PTR_ERR(d);
> -			goto err_closedir;
> +			goto err_cleanup;
>   		}
>   
>   		ret = snprintf(buf, PATH_MAX, "%s/%s", dir->i_srcpath, d->name);
>   		if (ret < 0 || ret >= PATH_MAX)
> -			goto err_closedir;
> +			goto err_cleanup;
>   
>   		inode = erofs_iget_from_srcpath(sbi, buf);
>   		if (IS_ERR(inode)) {
>   			ret = PTR_ERR(inode);
> -			goto err_closedir;
> +			goto err_cleanup;
>   		}
>   		d->inode = inode;
>   		d->type = erofs_mode_to_ftype(inode->i_mode);
> @@ -1484,7 +1472,7 @@ static int erofs_mkfs_handle_directory(struct erofs_inode *dir)
>   		erofs_dbg("file %s added (type %u)", buf, d->type);
>   		nr_subdirs++;
>   	}
> -	closedir(_dir);
> +	free(dent);
>   
>   	ret = erofs_init_empty_dir(dir);
>   	if (ret)
> @@ -1506,8 +1494,9 @@ static int erofs_mkfs_handle_directory(struct erofs_inode *dir)
>   
>   	return erofs_mkfs_go(sbi, EROFS_MKFS_JOB_DIR, &dir, sizeof(dir));
>   
> -err_closedir:
> -	closedir(_dir);
> +err_cleanup:
> +	for (; i < num_entries; free(dent[i]), i++);
> +	free(dent);
>   	return ret;
>   }
>