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

[Hongzhen Luo]

On 2024/9/3 15:41, Gao Xiang wrote:
>
>
> 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.
>
Sure, I will do this in the next patch.
>>
>> Signed-off-by: Hongzhen Luo <hongzhen at 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?
My original intention was to think that `goto out;` and `break;` were 
equivalent here. But, yes, I will make changes in the next version.
>
>> +        if (ret) {
>> +            size = ret;
>> +            value = malloc(size);
>> +            if (!value) {
>> +                ret = -ENOMEM;
>> +                goto out;
>
> same here.
Ditto.
>
>> +            }
>> +            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;
>> +}

---

Thanks,

Hongzhen Luo