Re: [PATCH] erofs-utils: fsck: add support for extracting hard links

[Yue Hu <zbestahu@gmail.com>]

On Tue, 27 Jun 2023 17:47:33 +0800
Gao Xiang <hsiangkao@linux.alibaba.com> wrote:

> Hi Yue,
> 
> On 2023/6/27 16:53, Yue Hu wrote:
> > From: Yue Hu <huyue2@coolpad.com>
> > 
> > Currently hard links can't be extracted correctly, let's support it now.
> > 
> > Signed-off-by: Yue Hu <huyue2@coolpad.com>  
> 
> Thanks for the patch!  Some minor comments below...
> 
> > ---
> >   fsck/main.c | 152 ++++++++++++++++++++++++++++++++++++++++++----------
> >   1 file changed, 124 insertions(+), 28 deletions(-)
> > 
> > diff --git a/fsck/main.c b/fsck/main.c
> > index f816bec..e78f67c 100644
> > --- a/fsck/main.c
> > +++ b/fsck/main.c
> > @@ -49,6 +49,16 @@ static struct option long_options[] = {
> >   	{0, 0, 0, 0},
> >   };
> >   
> > +#define NR_HARDLINK_HASHTABLE	16384
> > +
> > +struct hardlink_entry {  
> 
> erofsfsck_hardlink_entry {
> 
> or erofsfsck_linkentry if the above is too long
> 
> > +	struct list_head list;
> > +	erofs_nid_t nid;
> > +	char *path;
> > +};
> > +
> > +static struct list_head hardlink_hashtable[NR_HARDLINK_HASHTABLE];  
> 
> erofsfsck_link_hashtable
> 
> > +
> >   static void print_available_decompressors(FILE *f, const char *delim)
> >   {
> >   	unsigned int i = 0;
> > @@ -550,6 +560,64 @@ static inline int erofs_extract_dir(struct erofs_inode *inode)
> >   	return 0;
> >   }
> >   
> > +static char *erofsfsck_hardlink_find(erofs_nid_t nid)
> > +{
> > +	struct list_head *head =
> > +			&hardlink_hashtable[nid % NR_HARDLINK_HASHTABLE];
> > +	struct hardlink_entry *entry;
> > +
> > +	if (list_empty(head))
> > +		return NULL;  
> 
> why we need that?

yeah, it's unneeded, will remove it in v2.

> 
> > +
> > +	list_for_each_entry(entry, head, list)
> > +		if (entry->nid == nid)
> > +			return entry->path;
> > +	return NULL;
> > +}
> > +
> > +static int erofsfsck_hardlink_insert(erofs_nid_t nid, const char *path)
> > +{
> > +	struct hardlink_entry *entry;
> > +
> > +	entry = malloc(sizeof(*entry));
> > +	if (!entry)
> > +		return -ENOMEM;
> > +
> > +	entry->nid = nid;
> > +	entry->path = strdup(path);
> > +	if (!entry->path)
> > +		return -ENOMEM;
> > +
> > +	list_add_tail(&entry->list,
> > +		      &hardlink_hashtable[nid % NR_HARDLINK_HASHTABLE]);
> > +	return 0;
> > +}
> > +
> > +static void erofsfsck_hardlink_init(void)
> > +{
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < NR_HARDLINK_HASHTABLE; ++i)
> > +		init_list_head(&hardlink_hashtable[i]);
> > +}
> > +
> > +static void erofsfsck_hardlink_exit(void)
> > +{
> > +	struct hardlink_entry *entry, *n;
> > +	struct list_head *head;
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < NR_HARDLINK_HASHTABLE; ++i) {
> > +		head = &hardlink_hashtable[i];
> > +
> > +		list_for_each_entry_safe(entry, n, head, list) {
> > +			if (entry->path)
> > +				free(entry->path);
> > +			free(entry);
> > +		}
> > +	}
> > +}
> > +
> >   static inline int erofs_extract_file(struct erofs_inode *inode)
> >   {
> >   	bool tryagain = true;
> > @@ -719,6 +787,57 @@ static int erofsfsck_dirent_iter(struct erofs_dir_context *ctx)
> >   	return ret;
> >   }
> >   
> > +static int erofsfsck_extract_inode(struct erofs_inode *inode)
> > +{
> > +	int ret;
> > +	char *oldpath;
> > +
> > +	if (!fsckcfg.extract_path || erofs_is_packed_inode(inode)) {  
> 
> why introducing erofs_is_packed_inode here?

hardlink hashtable will be initialized after verify packed inode. 

> 
> Thanks,
> Gao Xiang