Re: [PATCH v1] erofs-utils: mkfs: fix fingerprint not set in certain modes

On 2026/4/10 17:00, [email protected] wrote:
>> On 2026/4/10 14:05, Yuezhang Mo wrote:
>>> In certain modes, such as "--tar=f --sort=none", data is written to
>>> the image before fingerprint calculation. In this case, ->datasource
>>> will be set to `EROFS_INODE_DATA_SOURCE_NONE`.
>>>
>>> The original `erofs_set_inode_fingerprint()` function only attempts to
>>> read data from a local file or disk buffer; it cannot handle the
>>> `EROFS_INODE_DATA_SOURCE_NONE` case, causing fingerprint setting to be
>>> skipped.
>>>
>>> This patch adds handling for the `EROFS_INODE_DATA_SOURCE_NONE` case,
>>> reading data from the image and calculating the fingerprint.
>>>
>>> Signed-off-by: Yuezhang Mo <[email protected]>
>>> Reviewed-by: Friendy Su <[email protected]>
>>> Reviewed-by: Daniel Palmer <[email protected]>
>>> ---
>>>    lib/inode.c | 20 ++++++++++++++------
>>>    1 file changed, 14 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/lib/inode.c b/lib/inode.c
>>> index 2cfc6c5..51d5266 100644
>>> --- a/lib/inode.c
>>> +++ b/lib/inode.c
>>> @@ -1975,6 +1975,13 @@ static int erofs_set_inode_fingerprint(struct erofs_inode *inode, int fd,
>>>
>>>        if (!ishare_xattr_prefix_id)
>>>                return 0;
>>> +
>>> +     if (inode->datasource == EROFS_INODE_DATA_SOURCE_NONE) {
>>> +             ret = erofs_iopen(&vf, inode);
>>> +             if (ret)
>>> +                     return ret;
>>> +     }
>>> +
>>>        erofs_sha256_init(&md);
>>>        do {
>>>                u8 buf[32768];
>>> @@ -2018,12 +2025,6 @@ static int erofs_mkfs_begin_nondirectory(const struct erofs_mkfs_btctx *btctx,
>>>                        goto out;
>>>                }
>>>
>>> -             if (S_ISREG(inode->i_mode) && inode->i_size) {
>>> -                     ret = erofs_set_inode_fingerprint(inode, ctx.fd, ctx.fpos);
>>> -                     if (ret < 0)
>>> -                             return ret;
>>> -             }
>>
>> I vaguely remembered we have to leave it here since
>> otherwise it may impact compressed files.
>>
>> Also EROFS_INODE_DATA_SOURCE_NONE means that mkfs
>> dump will change nothing about that, so I suggest
>>
>> apply erofs_set_inode_fingerprint() to every
>> EROFS_INODE_DATA_SOURCE_NONE user
>> (e.g. in lib/tar.c) instead.
>>
> 
> Hi Xiang,
> 
> This will bring a bit big code change.
> 
> How about changing it like this? It maintains the original
> order and the change is simple.

`EROFS_INODE_DATA_SOURCE_NONE` should assume that metadata
is done as-is, that is the current design principle.

using erofs_iopen() to reuse erofs_pread() is really
unexpected for me TBH.

Thanks,
Gao Xiang