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

[Gao Xiang <hsiangkao@linux.alibaba.com>]

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?

> +
> +	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?

Thanks,
Gao Xiang