On Sat, Dec 05, 2020 at 05:05:35PM +0800, Li GuiFu 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); > Ok, how about the following statement: DBG_BUGON(ftype == EROFS_FT_DIR && d->type != ftype); It will save some words. I will send the next version soon. Thanks, Gao Xiang
