Re: [PATCH] erofs-utils: fsck: add --xattr option

[Gao Xiang]

On 2024/9/3 15:35, Hongzhen Luo wrote:

The current `fsck --extract` does not support exporting the extended
attributes of files. This patch adds `--xattr` option to dump the
extended attributes.

only `--xattrs` is not helpful, you'd better to add both
`--xattrs` and `--no-xattrs`, and makes `--xattrs` enabled
by default.

Signed-off-by: Hongzhen Luo <hongz...@linux.alibaba.com>
---
  fsck/main.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 61 insertions(+)

diff --git a/fsck/main.c b/fsck/main.c
index 28f1e7e..6a71791 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -9,6 +9,7 @@
  #include <utime.h>
  #include <unistd.h>
  #include <sys/stat.h>
+#include <sys/xattr.h>
  #include "erofs/print.h"
  #include "erofs/compress.h"
  #include "erofs/decompress.h"
@@ -31,6 +32,7 @@ struct erofsfsck_cfg {
        bool overwrite;
        bool preserve_owner;
        bool preserve_perms;
+       bool xattr;

        bool dump_xattrs;

  };
  static struct erofsfsck_cfg fsckcfg;

@@ -48,6 +50,7 @@ static struct option long_options[] = {

        {"no-preserve-owner", no_argument, 0, 10},
        {"no-preserve-perms", no_argument, 0, 11},
        {"offset", required_argument, 0, 12},
+       {"xattr", no_argument, 0, 13},

--xattrs, --no-xattrs

        {0, 0, 0, 0},
  };

@@ -98,6 +101,7 @@ static void usage(int argc, char **argv)

                " --extract[=X]          check if all files are well encoded, 
optionally\n"
                "                        extract to X\n"
                " --offset=#             skip # bytes at the beginning of 
IMAGE\n"
+               " --xattr                dump extended attributes\n"
                "\n"
                " -a, -A, -y             no-op, for compatibility with fsck of other 
filesystems\n"
                "\n"
@@ -225,6 +229,9 @@ static int erofsfsck_parse_options_cfg(int argc, char 
**argv)
                                return -EINVAL;
                        }
                        break;
+               case 13:
+                       fsckcfg.xattr = true;
+                       break;
                default:
                        return -EINVAL;
                }
@@ -411,6 +418,53 @@ out:
        return ret;
  }

+static int erofs_dump_xattr(struct erofs_inode *inode)

erofsfsck_dump_xattrs

+{
+       char *keylst, *key;
+       ssize_t kllen;
+       int ret;
+
+       if (!fsckcfg.extract_path)

If extract_path is none, please keep reading the xattrs
but don't dump these.

+               return 0;
+       kllen = erofs_listxattr(inode, NULL, 0);
+       if (kllen <= 0)
+               return kllen;
+       keylst = malloc(kllen);
+       if (!keylst)
+               return -ENOMEM;
+       ret = erofs_listxattr(inode, keylst, kllen);
+       if (ret < 0)
+               goto out;
+       for (key = keylst; key < keylst + kllen; key += strlen(key) + 1) {
+               void *value = NULL;
+               size_t size = 0;
+
+               ret = erofs_getxattr(inode, key, NULL, 0);
+               if (ret < 0)
+                       goto out;

why not just `break;` this?

+               if (ret) {
+                       size = ret;
+                       value = malloc(size);
+                       if (!value) {
+                               ret = -ENOMEM;
+                               goto out;

same here.

+                       }
+                       ret = erofs_getxattr(inode, key, value, size);
+                       if (ret < 0) {
+                               free(value);
+                               goto out;

same here.

+                       }
+                       ret = setxattr(fsckcfg.extract_path, key, value, size, 
0);
+                       free(value);
+                       if (ret)
+                               goto out;

same here.

Thanks,
Gao Xiang

+               }
+       }
+out:
+       free(keylst);
+       return ret;
+}