Re: [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 <jooy...@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-jooy...@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;
  }