Re: [PATCH] erofs-utils: lib: add support for symlink file in erofs_ilookup()

[Hongzhen Luo <hongzhen@linux.alibaba.com>]

On 2024/8/12 10:24, Gao Xiang wrote:
>
>
> On 2024/8/11 16:09, Hongzhen Luo wrote:
>> When the `path` contains symbolic links, erofs_ilookup() does not
>> function properly. This adds support for symlink files.
>
> Can you explain what's the use cases of this patch?
>
> It seems both erofsfuse and fsck.erofs --extract don't need this.
>
Some third-party applications (such as Alibaba DADI) require obtaining 
block mapping information of files based on their paths using liberofs. 
When file paths include symbolic links, the current erofs_ilookup() 
function fails to correctly locate the inode. This submission enhances 
erofs_ilookup()'s support for symbolic link files.
>>
>> Signed-off-by: Hongzhen Luo <hongzhen@linux.alibaba.com>
>> ---
>>   lib/namei.c | 25 ++++++++++++++++++++++++-
>>   1 file changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/namei.c b/lib/namei.c
>> index 6f35ee6..dce2991 100644
>> --- a/lib/namei.c
>> +++ b/lib/namei.c
>> @@ -195,6 +195,22 @@ struct nameidata {
>>       unsigned int    ftype;
>>   };
>>   +static int link_path_walk(const char *name, struct nameidata *nd);
>> +
>> +static int step_into_link(struct nameidata *nd, struct erofs_inode *vi)
>> +{
>> +    char buf[EROFS_MAX_BLOCK_SIZE];
>> +    int err;
>> +
>> +    if (vi->i_size > EROFS_MAX_BLOCK_SIZE)
>> +        return -EINVAL;
>
> No, symlink size is independent to EROFS_MAX_BLOCK_SIZE, currently
> it's hard-code as 4096.
>
Okay, I will make the corresponding modifications in the next version.
>> +    memset(buf, 0, sizeof(buf));
>> +    err = erofs_pread(vi, buf, vi->i_size, 0);
>> +    if (err)
>> +        return err;
>> +    return link_path_walk(buf, nd);
>> +}
>> +
>>   int erofs_namei(struct nameidata *nd, const char *name, unsigned 
>> int len)
>>   {
>>       erofs_nid_t nid = nd->nid;
>> @@ -233,6 +249,11 @@ int erofs_namei(struct nameidata *nd, const char 
>> *name, unsigned int len)
>>               return PTR_ERR(de);
>>             if (de) {
>> +            vi.nid = de->nid;
>> +            ret = erofs_read_inode_from_disk(&vi);
>> +            if (S_ISLNK(vi.i_mode)) {
>> +                return step_into_link(nd, &vi);
>> +            }
>
> Why need brace here?

Yes, the braces are not necessary, and I will clean them up later.

>
> Thanks,
> Gao Xiang

---

Thanks,

Hongzhen Luo