Re: [PATCH] erofs-utils: update i_nlink stat for directories

[Li GuiFu via Linux-erofs <linux-erofs@lists.ozlabs.org>]

On 2020/12/5 16:43, Gao Xiang wrote:
> On Sat, Dec 05, 2020 at 04:38:37PM +0800, Gao Xiang wrote:
>> On Sat, Dec 05, 2020 at 04:32:44PM +0800, Li GuiFu via Linux-erofs wrote:
>>
>> ...
>>
>>>
>>>> @@ -957,6 +974,10 @@ struct erofs_inode *erofs_mkfs_build_tree(struct erofs_inode *dir)
>>>>  			ret = PTR_ERR(d);
>>>>  			goto err_closedir;
>>>>  		}
>>>> +
>>>> +		/* to count i_nlink for directories */
>>>> +		d->type = (dp->d_type == DT_DIR ?
>>>> +			EROFS_FT_DIR : EROFS_FT_UNKNOWN);
>>>>  	}
>>>>  
>>> It's confused that d->type was set to EROFS_FT_UNKNOWN when not a dir
>>> It's not clearness whether the program goes wrong or get the wrong data
>>> Actually it's a correct procedure
>>
>> It's just set temporarily, since only dirs are useful when counting subdirs, so
>> only needs to differ dirs and non-dirs here. (Previously d->type is unused
>> at this time.)
> 
> btw, I once tried to set up d->type via dp->d_type here, but it increases a
> lot of code and seems unnecessary (since deriving from i_mode is enough).
> So again, here we only cares about dir and non-dirs (we don't care much about
> the specific kind of non-dirs here).
> 
> Thanks,
> Gao Xiang
> 
>>
>> ...
>>
>>>> -		d->type = erofs_type_by_mode[d->inode->i_mode >> S_SHIFT];
>>>> +		ftype = erofs_mode_to_ftype(d->inode->i_mode);
>>>> +		DBG_BUGON(d->type != EROFS_FT_UNKNOWN && d->type != ftype);
>>>> +		d->type = ftype;
>>
>> The real on-disk d->type will be set here rather than the above.
Yes, what it makes confused is here, EROFS_FT_UNKNOWN is just temporary.
So how about change to ASSERT at EROFS_FT_DIR

DBG_BUGON(d->type == EROFS_FT_DIR && ftype != EROFS_FT_DIR);