Re: [PATCH] erofs-utils: dump: Add --cat flag to show file contents

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

Hi Juan,

On 2025/1/9 17:55, Juan Hernandez wrote:
> This patch adds a new '--cat' flag to the 'dump.erofs' command. When
> used it will write to the standard output the content of the file
> indicated by the '--path' or '--nid' options. For example, if there is a
> '/mydir/myfile.txt' file containg the text 'mytext':
> 
>      $ dump.erofs --cat --path=/mydir/myfile.txt myimage.erofs
>      mytext
> 
> Signed-off-by: Juan Hernandez <jhernand@redhat.com>

Thanks for the patch!
It looks good to me, just minor nits..

> ---
>   dump/main.c      | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
>   man/dump.erofs.1 | 13 +++++++--
>   2 files changed, 83 insertions(+), 2 deletions(-)
> 
> diff --git a/dump/main.c b/dump/main.c
> index 372162e..ed8d44d 100644
> --- a/dump/main.c
> +++ b/dump/main.c
> @@ -26,6 +26,7 @@ struct erofsdump_cfg {
>   	bool show_superblock;
>   	bool show_statistics;
>   	bool show_subdirectories;
> +	bool show_file_content;
>   	erofs_nid_t nid;
>   	const char *inode_path;
>   };
> @@ -80,6 +81,7 @@ static struct option long_options[] = {
>   	{"path", required_argument, NULL, 4},
>   	{"ls", no_argument, NULL, 5},
>   	{"offset", required_argument, NULL, 6},
> +	{"cat", no_argument, NULL, 7},
>   	{0, 0, 0, 0},
>   };
>   
> @@ -123,6 +125,7 @@ static void usage(int argc, char **argv)
>   		" -s              show information about superblock\n"
>   		" --device=X      specify an extra device to be used together\n"
>   		" --ls            show directory contents (INODE required)\n"
> +		" --cat           show file contents (INODE required)\n"
>   		" --nid=#         show the target inode info of nid #\n"
>   		" --offset=#      skip # bytes at the beginning of IMAGE\n"
>   		" --path=X        show the target inode info of path X\n",
> @@ -186,6 +189,9 @@ static int erofsdump_parse_options_cfg(int argc, char **argv)
>   				return -EINVAL;
>   			}
>   			break;
> +		case 7:
> +			dumpcfg.show_file_content = true;
> +			break;
>   		default:
>   			return -EINVAL;
>   		}
> @@ -672,6 +678,63 @@ static void erofsdump_show_superblock(void)
>   			uuid_str);
>   }
>   
> +static void erofsdump_show_file_content(void)
> +{
> +	int err;
> +	struct erofs_inode inode = { .sbi = &g_sbi, .nid = dumpcfg.nid };
> +	size_t buffer_size;
> +	char *buffer_ptr;
> +	erofs_off_t pending_size;
> +	erofs_off_t read_offset;
> +	erofs_off_t read_size;
> +
> +	if (dumpcfg.inode_path) {
> +		err = erofs_ilookup(dumpcfg.inode_path, &inode);
> +		if (err) {
> +			erofs_err("read inode failed @ %s", dumpcfg.inode_path);
> +			return;
> +		}
> +	} else {
> +		err = erofs_read_inode_from_disk(&inode);
> +		if (err) {
> +			erofs_err("read inode failed @ nid %llu", inode.nid | 0ULL);
> +			return;
> +		}
> +	}
> +
> +	if (!S_ISREG(inode.i_mode)) {

I think we could dump raw directory content too.


> +		erofs_err("not a regular file @ nid %llu", inode.nid | 0ULL);
> +		return;
> +	}
> +
> +	buffer_size = erofs_blksiz(inode.sbi);
> +	buffer_ptr = malloc(buffer_size);
> +	if (!buffer_ptr) {
> +		erofs_err("buffer allocation failed @ nid %llu", inode.nid | 0ULL);
> +		return;
> +	}
> +
> +	pending_size = inode.i_size;
> +	read_offset = 0;
> +	while (pending_size > 0) {
> +		read_size = pending_size > buffer_size? buffer_size: pending_size;
> +		err = erofs_pread(&inode, buffer_ptr, read_size, read_offset);
> +		if (err) {
> +			erofs_err("read file failed @ nid %llu", inode.nid | 0ULL);
> +			goto out;
> +		}
> +		pending_size -= read_size;
> +		read_offset += read_size;
> +		fwrite(buffer_ptr, read_size, 1, stdout);
> +	}
> +	fflush(stdout);
> +
> +out:
> +	free(buffer_ptr);
> +}
> +
> +
> +

Reduntant new lines..

Thanks,
Gao Xiang