On Sun, May 09, 2021 at 10:30:06PM +0800, Li GuiFu wrote: > GaoXiang > > On 2021/5/7 15:09, Gao Xiang wrote: > > Hi Guifu, > > > > On Wed, May 05, 2021 at 10:26:15PM +0800, Li Guifu via Linux-erofs wrote: > >> The structure erofs_subdirs has a dir number and a list_head, > >> the list_head is the same with i_subdirs in the inode. > >> Using erofs_subdirs as a temp place for dentrys under the dir, > >> and then sort it before replace to i_subdirs > >> > >> Signed-off-by: Li Guifu <[email protected]> > >> --- > >> include/erofs/internal.h | 5 +++ > >> lib/inode.c | 95 +++++++++++++++++++++++++--------------- > >> 2 files changed, 64 insertions(+), 36 deletions(-) > >> > >> diff --git a/include/erofs/internal.h b/include/erofs/internal.h > >> index 1339341..7cd42ca 100644 > >> --- a/include/erofs/internal.h > >> +++ b/include/erofs/internal.h > >> @@ -172,6 +172,11 @@ struct erofs_inode { > >> #endif > >> }; > >> > >> +struct erofs_subdirs { > >> + struct list_head i_subdirs; > >> + unsigned int nr_subdirs; > >> +}; > >> + > >> static inline bool is_inode_layout_compression(struct erofs_inode *inode) > >> { > >> return erofs_inode_is_data_compressed(inode->datalayout); > >> diff --git a/lib/inode.c b/lib/inode.c > >> index 787e5b4..3e138a6 100644 > >> --- a/lib/inode.c > >> +++ b/lib/inode.c > >> @@ -96,7 +96,7 @@ unsigned int erofs_iput(struct erofs_inode *inode) > >> return 0; > >> } > >> > >> -struct erofs_dentry *erofs_d_alloc(struct erofs_inode *parent, > >> +struct erofs_dentry *erofs_d_alloc(struct erofs_subdirs *subdirs, > >> const char *name) > >> { > >> struct erofs_dentry *d = malloc(sizeof(*d)); > >> @@ -107,7 +107,8 @@ struct erofs_dentry *erofs_d_alloc(struct erofs_inode > >> *parent, > >> strncpy(d->name, name, EROFS_NAME_LEN - 1); > >> d->name[EROFS_NAME_LEN - 1] = '\0'; > >> > >> - list_add_tail(&d->d_child, &parent->i_subdirs); > >> + list_add_tail(&d->d_child, &subdirs->i_subdirs); > >> + subdirs->nr_subdirs++; > >> return d; > >> } > >> > >> @@ -150,38 +151,12 @@ static int comp_subdir(const void *a, const void *b) > >> return strcmp(da->name, db->name); > >> } > >> > >> -int erofs_prepare_dir_file(struct erofs_inode *dir, unsigned int > >> nr_subdirs) > >> +int erofs_prepare_dir_file(struct erofs_inode *dir) > > > > Err... nope, that is not what I suggested, I was suggesting > > > > int erofs_prepare_dir_file(struct erofs_inode *dir, > > struct erofs_subdirs *subdirs) > > > >> { > >> - struct erofs_dentry *d, *n, **sorted_d; > >> - unsigned int d_size, i_nlink, i; > >> + struct erofs_dentry *d; > >> + unsigned int d_size, i_nlink; > >> int ret; > >> > >> - /* dot is pointed to the current dir inode */ > >> - d = erofs_d_alloc(dir, "."); > >> - d->inode = erofs_igrab(dir); > >> - d->type = EROFS_FT_DIR; > >> - > >> - /* dotdot is pointed to the parent dir */ > >> - d = erofs_d_alloc(dir, ".."); > >> - d->inode = erofs_igrab(dir->i_parent); > >> - d->type = EROFS_FT_DIR; > > > > Leave these two here, since it's a part of dir preparation. > I think this erofs_prepare_dir_file() funtion just do space prepare > that dirs have been sorted before > > . and .. should been added along with readir
Nope, I don't think introduce another two one-shot fixed helpers would be helpful here. Also, prepare_dir_file() clearly means "prepare for the dir file". It can naturally accept all original subdir inputs, add ".", ".." dirents, sort them all in dir formats and output the dir files. I don't know what is wrong with such clean sematics? And you must seperate it into several helpers? Thanks, Gao Xiang > >
