Re: [PATCH v2 1/3] erofs-utils: lib: export parts of erofs_pread()

[Yue Hu <zbestahu@gmail.com>]

On Tue, 10 Jan 2023 17:00:33 +0800
Xiang Gao <hsiangkao@linux.alibaba.com> wrote:

> Hi Yue,
> 
> The patch itself generally looks good to me:
> 
> On 2023/1/10 16:49, Yue Hu wrote:
> > From: Yue Hu <huyue2@coolpad.com>
> > 
> > Export parts of erofs_pread() to avoid duplicated code in
> > erofs_verify_inode_data(). Let's make two helpers for this.
> > 
> > Signed-off-by: Yue Hu <huyue2@coolpad.com>
> > ---
> > v2: use parameter trimmed instead of partial
> > 
> >   include/erofs/internal.h |   5 ++
> >   lib/data.c               | 154 ++++++++++++++++++++++-----------------
> >   2 files changed, 92 insertions(+), 67 deletions(-)
> > 
> > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > index 206913c..47240f5 100644
> > --- a/include/erofs/internal.h
> > +++ b/include/erofs/internal.h
> > @@ -355,6 +355,11 @@ int erofs_pread(struct erofs_inode *inode, char *buf,
> >   int erofs_map_blocks(struct erofs_inode *inode,
> >   		struct erofs_map_blocks *map, int flags);
> >   int erofs_map_dev(struct erofs_sb_info *sbi, struct erofs_map_dev *map);
> > +int erofs_read_raw_data_mapped(struct erofs_map_blocks *map, char *buffer,
> > +			       u64 offset, size_t len);
> > +int z_erofs_read_data_mapped(struct erofs_inode *inode,
> > +			struct erofs_map_blocks *map, char *raw, char *buffer,
> > +			erofs_off_t skip, erofs_off_t length, bool trimmed);
> >   
> >   static inline int erofs_get_occupied_size(const struct erofs_inode *inode,
> >   					  erofs_off_t *size)
> > diff --git a/lib/data.c b/lib/data.c
> > index 76a6677..d8c6076 100644
> > --- a/lib/data.c
> > +++ b/lib/data.c
> > @@ -158,19 +158,38 @@ int erofs_map_dev(struct erofs_sb_info *sbi, struct erofs_map_dev *map)
> >   	return 0;
> >   }
> >   
> > +int erofs_read_raw_data_mapped(struct erofs_map_blocks *map, char *buffer,  
> 
> erofs_read_one_data?
> 
> > +				u64 offset, size_t len)
> > +{
> > +	struct erofs_map_dev mdev;
> > +	int ret;
> > +
> > +	mdev = (struct erofs_map_dev) {
> > +		.m_deviceid = map->m_deviceid,
> > +		.m_pa = map->m_pa,
> > +	};
> > +	ret = erofs_map_dev(&sbi, &mdev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = dev_read(mdev.m_deviceid, buffer, mdev.m_pa + offset, len);
> > +	if (ret < 0)
> > +		return -EIO;
> > +	return 0;
> > +}
> > +
> >   static int erofs_read_raw_data(struct erofs_inode *inode, char *buffer,
> >   			       erofs_off_t size, erofs_off_t offset)
> >   {
> >   	struct erofs_map_blocks map = {
> >   		.index = UINT_MAX,
> >   	};
> > -	struct erofs_map_dev mdev;
> >   	int ret;
> >   	erofs_off_t ptr = offset;
> >   
> >   	while (ptr < offset + size) {
> >   		char *const estart = buffer + ptr - offset;
> > -		erofs_off_t eend;
> > +		erofs_off_t eend, moff = 0;
> >   
> >   		map.m_la = ptr;
> >   		ret = erofs_map_blocks(inode, &map, 0);
> > @@ -179,14 +198,6 @@ static int erofs_read_raw_data(struct erofs_inode *inode, char *buffer,
> >   
> >   		DBG_BUGON(map.m_plen != map.m_llen);
> >   
> > -		mdev = (struct erofs_map_dev) {
> > -			.m_deviceid = map.m_deviceid,
> > -			.m_pa = map.m_pa,
> > -		};
> > -		ret = erofs_map_dev(&sbi, &mdev);
> > -		if (ret)
> > -			return ret;
> > -
> >   		/* trim extent */
> >   		eend = min(offset + size, map.m_la + map.m_llen);
> >   		DBG_BUGON(ptr < map.m_la);
> > @@ -204,19 +215,74 @@ static int erofs_read_raw_data(struct erofs_inode *inode, char *buffer,
> >   		}
> >   
> >   		if (ptr > map.m_la) {
> > -			mdev.m_pa += ptr - map.m_la;
> > +			moff = ptr - map.m_la;
> >   			map.m_la = ptr;
> >   		}
> >   
> > -		ret = dev_read(mdev.m_deviceid, estart, mdev.m_pa,
> > -			       eend - map.m_la);
> > -		if (ret < 0)
> > -			return -EIO;
> > +		ret = erofs_read_raw_data_mapped(&map, estart, moff,
> > +						 eend - map.m_la);
> > +		if (ret)
> > +			return ret;
> >   		ptr = eend;
> >   	}
> >   	return 0;
> >   }
> >   
> > +int z_erofs_read_data_mapped(struct erofs_inode *inode,  
> 
> z_erofs_read_one_data?

Okay. Let me update these in v3.

> 
> Thanks,
> Gao Xiang