[PATCH] erofs-utils: fix the erofs_io_pread and erofs_io_pwrite

Gao Xiang hsiangkao at linux.alibaba.com
Mon Jun 17 13:41:50 AEST 2024



On 2024/6/17 10:34, Hongzhen Luo wrote:
> When `vf->ops` is not null, `vf->ops->pread` returns the
> number of bytes successfully read, which is inconsistent
> with the semantics of `erofs_io_pread`. Similar situation
> occurs in `erofs_io_pwrite`. This fixes this issue.
> 
> Signed-off-by: Hongzhen Luo <hongzhen at linux.alibaba.com>

I think virtual files don't need to handle short read/write
(or they need to handle those themselves.)


> ---
>   lib/io.c | 26 ++++++++++++++++----------
>   1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/io.c b/lib/io.c
> index c523f00..52a74dc 100644
> --- a/lib/io.c
> +++ b/lib/io.c
> @@ -34,15 +34,18 @@ ssize_t erofs_io_pwrite(struct erofs_vfile *vf, const void *buf,
>   	if (unlikely(cfg.c_dry_run))
>   		return 0;

But I think we might need to fix those return values for
non-virtual files (return the written bytes.)

>   
> -	if (vf->ops)
> -		return vf->ops->pwrite(vf, buf, pos, len);
> -
>   	pos += vf->offset;
>   	do {
>   #ifdef HAVE_PWRITE64
> -		ret = pwrite64(vf->fd, buf, len, (off64_t)pos);
> +		if (vf->ops)
> +			ret = vf->ops->pwrite(vf, buf, pos, len);
> +		else
> +			ret = pwrite64(vf->fd, buf, len, (off64_t)pos);
>   #else
> -		ret = pwrite(vf->fd, buf, len, (off_t)pos);
> +		if (vf->ops)
> +			ret = vf->ops->pwrite(vf, buf, pos, len);
> +		else
> +			ret = pwrite(vf->fd, buf, len, (off_t)pos);
>   #endif
>   		if (ret <= 0) {
>   			erofs_err("failed to write: %s", strerror(errno));
> @@ -130,15 +133,18 @@ ssize_t erofs_io_pread(struct erofs_vfile *vf, void *buf, u64 pos, size_t len)
>   	if (unlikely(cfg.c_dry_run))
>   		return 0;

Same here.

Thanks,
Gao Xiang

>   
> -	if (vf->ops)
> -		return vf->ops->pread(vf, buf, pos, len);
> -
>   	pos += vf->offset;
>   	do {
>   #ifdef HAVE_PREAD64
> -		ret = pread64(vf->fd, buf, len, (off64_t)pos);
> +		if (vf->ops)
> +			ret = vf->ops->pread(vf, buf, pos, len);
> +		else
> +			ret = pread64(vf->fd, buf, len, (off64_t)pos);
>   #else
> -		ret = pread(vf->fd, buf, len, (off_t)pos);
> +		if (vf->ops)
> +			ret = vf->ops->pread(vf, buf, pos, len);
> +		else
> +			ret = pread(vf->fd, buf, len, (off_t)pos);
>   #endif
>   		if (ret <= 0) {
>   			if (!ret) {


More information about the Linux-erofs mailing list