All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] erofs-utils:code clean of write file
@ 2019-12-18 15:52 Li Guifu
  2019-12-19 18:19 ` Gao Xiang via Linux-erofs
  0 siblings, 1 reply; 2+ messages in thread
From: Li Guifu @ 2019-12-18 15:52 UTC (permalink / raw)
  To: linux-erofs

From: Li Guifu <bluce.liguifu@huawei.com>

Make a code clean at function erofs_write_file() which
has multi jump.

Signed-off-by: Li Guifu <blucerlee@gmail.com>
---
 lib/inode.c | 63 ++++++++++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 32 deletions(-)

diff --git a/lib/inode.c b/lib/inode.c
index 0e19b11..052315a 100644
--- a/lib/inode.c
+++ b/lib/inode.c
@@ -302,22 +302,10 @@ static bool erofs_file_is_compressible(struct erofs_inode *inode)
 	return true;
 }
 
-int erofs_write_file(struct erofs_inode *inode)
+int erofs_write_file_by_fd(int fd, struct erofs_inode *inode)
 {
+	int ret;
 	unsigned int nblocks, i;
-	int ret, fd;
-
-	if (!inode->i_size) {
-		inode->datalayout = EROFS_INODE_FLAT_PLAIN;
-		return 0;
-	}
-
-	if (cfg.c_compr_alg_master && erofs_file_is_compressible(inode)) {
-		ret = erofs_write_compressed_file(inode);
-
-		if (!ret || ret != -ENOSPC)
-			return ret;
-	}
 
 	/* fallback to all data uncompressed */
 	inode->datalayout = EROFS_INODE_FLAT_INLINE;
@@ -327,47 +315,58 @@ int erofs_write_file(struct erofs_inode *inode)
 	if (ret)
 		return ret;
 
-	fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
-	if (fd < 0)
-		return -errno;
-
 	for (i = 0; i < nblocks; ++i) {
 		char buf[EROFS_BLKSIZ];
 
 		ret = read(fd, buf, EROFS_BLKSIZ);
-		if (ret != EROFS_BLKSIZ) {
-			if (ret < 0)
-				goto fail;
-			close(fd);
-			return -EAGAIN;
-		}
+		if (ret != EROFS_BLKSIZ)
+			return -errno;
 
 		ret = blk_write(buf, inode->u.i_blkaddr + i, 1);
 		if (ret)
-			goto fail;
+			return ret;
 	}
 
 	/* read the tail-end data */
 	inode->idata_size = inode->i_size % EROFS_BLKSIZ;
 	if (inode->idata_size) {
 		inode->idata = malloc(inode->idata_size);
-		if (!inode->idata) {
-			close(fd);
+		if (!inode->idata)
 			return -ENOMEM;
-		}
 
 		ret = read(fd, inode->idata, inode->idata_size);
 		if (ret < inode->idata_size) {
 			free(inode->idata);
 			inode->idata = NULL;
-			close(fd);
 			return -EIO;
 		}
 	}
-	close(fd);
+
 	return 0;
-fail:
-	ret = -errno;
+}
+
+int erofs_write_file(struct erofs_inode *inode)
+{
+	int ret, fd;
+
+	if (!inode->i_size) {
+		inode->datalayout = EROFS_INODE_FLAT_PLAIN;
+		return 0;
+	}
+
+	if (cfg.c_compr_alg_master && erofs_file_is_compressible(inode)) {
+		ret = erofs_write_compressed_file(inode);
+
+		if (!ret || ret != -ENOSPC)
+			return ret;
+	}
+
+	fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
+	if (fd < 0)
+		return -errno;
+
+	ret = erofs_write_file_by_fd(fd, inode);
+
 	close(fd);
 	return ret;
 }
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v1] erofs-utils:code clean of write file
  2019-12-18 15:52 [PATCH v1] erofs-utils:code clean of write file Li Guifu
@ 2019-12-19 18:19 ` Gao Xiang via Linux-erofs
  0 siblings, 0 replies; 2+ messages in thread
From: Gao Xiang via Linux-erofs @ 2019-12-19 18:19 UTC (permalink / raw)
  To: Li Guifu; +Cc: Miao Xie, linux-erofs

Hi Guifu,

Sorry for some delay, I'm busying in developping new fixed-sized output
LZMA library approach now (I think I'm fully understand LZMA internals,
hopefully in RFC shape and open source a preliminary effective
implementation next month, and I don't want to delay it too longer...)

Some changes are made as below (and a new version at the end of this
message and experimental branch). Comment as well if you have some
other opinions...

p.s. could you take some time looking at the requirement, thanks a lot!
https://lore.kernel.org/r/CAEvUa7=N7qUobof=vwpXF2XfXcW8R67SB3KV1phRN2ZmG23CvQ@mail.gmail.com/

On Wed, Dec 18, 2019 at 11:52:37PM +0800, Li Guifu wrote:
> From: Li Guifu <bluce.liguifu@huawei.com>
> 
> Make a code clean at function erofs_write_file() which
> has multi jump.

I've rewriten the commit message.

> 
> Signed-off-by: Li Guifu <blucerlee@gmail.com>
> ---
>  lib/inode.c | 63 ++++++++++++++++++++++++++---------------------------
>  1 file changed, 31 insertions(+), 32 deletions(-)
> 
> diff --git a/lib/inode.c b/lib/inode.c
> index 0e19b11..052315a 100644
> --- a/lib/inode.c
> +++ b/lib/inode.c
> @@ -302,22 +302,10 @@ static bool erofs_file_is_compressible(struct erofs_inode *inode)
>  	return true;
>  }
>  
> -int erofs_write_file(struct erofs_inode *inode)
> +int erofs_write_file_by_fd(int fd, struct erofs_inode *inode)

I've rename the function to write_uncompressed_file_from_fd and
change the variable order.

>  {
> +	int ret;
>  	unsigned int nblocks, i;
> -	int ret, fd;
> -
> -	if (!inode->i_size) {
> -		inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> -		return 0;
> -	}
> -
> -	if (cfg.c_compr_alg_master && erofs_file_is_compressible(inode)) {
> -		ret = erofs_write_compressed_file(inode);
> -
> -		if (!ret || ret != -ENOSPC)
> -			return ret;
> -	}
>  
>  	/* fallback to all data uncompressed */
>  	inode->datalayout = EROFS_INODE_FLAT_INLINE;
> @@ -327,47 +315,58 @@ int erofs_write_file(struct erofs_inode *inode)
>  	if (ret)
>  		return ret;
>  
> -	fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
> -	if (fd < 0)
> -		return -errno;
> -
>  	for (i = 0; i < nblocks; ++i) {
>  		char buf[EROFS_BLKSIZ];
>  
>  		ret = read(fd, buf, EROFS_BLKSIZ);
> -		if (ret != EROFS_BLKSIZ) {
> -			if (ret < 0)
> -				goto fail;
> -			close(fd);
> -			return -EAGAIN;
> -		}
> +		if (ret != EROFS_BLKSIZ)
> +			return -errno;

I'd suggest the original approach.

Thanks,
Gao Xiang

>  
>  		ret = blk_write(buf, inode->u.i_blkaddr + i, 1);
>  		if (ret)
> -			goto fail;
> +			return ret;
>  	}
>  
>  	/* read the tail-end data */
>  	inode->idata_size = inode->i_size % EROFS_BLKSIZ;
>  	if (inode->idata_size) {
>  		inode->idata = malloc(inode->idata_size);
> -		if (!inode->idata) {
> -			close(fd);
> +		if (!inode->idata)
>  			return -ENOMEM;
> -		}
>  
>  		ret = read(fd, inode->idata, inode->idata_size);
>  		if (ret < inode->idata_size) {
>  			free(inode->idata);
>  			inode->idata = NULL;
> -			close(fd);
>  			return -EIO;
>  		}
>  	}
> -	close(fd);
> +
>  	return 0;
> -fail:
> -	ret = -errno;
> +}
> +
> +int erofs_write_file(struct erofs_inode *inode)
> +{
> +	int ret, fd;
> +
> +	if (!inode->i_size) {
> +		inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> +		return 0;
> +	}
> +
> +	if (cfg.c_compr_alg_master && erofs_file_is_compressible(inode)) {
> +		ret = erofs_write_compressed_file(inode);
> +
> +		if (!ret || ret != -ENOSPC)
> +			return ret;
> +	}
> +
> +	fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
> +	if (fd < 0)
> +		return -errno;
> +
> +	ret = erofs_write_file_by_fd(fd, inode);
> +
>  	close(fd);
>  	return ret;
>  }
> -- 
> 2.17.1
> 


From 11302fc4dc5d53a7730405765828a744aee114f6 Mon Sep 17 00:00:00 2001
From: Li Guifu <blucerlee@gmail.com>
Date: Wed, 18 Dec 2019 23:52:37 +0800
Subject: [PATCH] erofs-utils: clean up erofs_write_file()

Introduce write_uncompressed_file_from_fd() to make
error handling path in erofs_write_file() clearer.

Signed-off-by: Li Guifu <blucerlee@gmail.com>
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 lib/inode.c | 58 ++++++++++++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/lib/inode.c b/lib/inode.c
index 0e19b11..bd0652b 100644
--- a/lib/inode.c
+++ b/lib/inode.c
@@ -302,24 +302,11 @@ static bool erofs_file_is_compressible(struct erofs_inode *inode)
 	return true;
 }
 
-int erofs_write_file(struct erofs_inode *inode)
+static int write_uncompressed_file_from_fd(struct erofs_inode *inode, int fd)
 {
+	int ret;
 	unsigned int nblocks, i;
-	int ret, fd;
 
-	if (!inode->i_size) {
-		inode->datalayout = EROFS_INODE_FLAT_PLAIN;
-		return 0;
-	}
-
-	if (cfg.c_compr_alg_master && erofs_file_is_compressible(inode)) {
-		ret = erofs_write_compressed_file(inode);
-
-		if (!ret || ret != -ENOSPC)
-			return ret;
-	}
-
-	/* fallback to all data uncompressed */
 	inode->datalayout = EROFS_INODE_FLAT_INLINE;
 	nblocks = inode->i_size / EROFS_BLKSIZ;
 
@@ -327,47 +314,60 @@ int erofs_write_file(struct erofs_inode *inode)
 	if (ret)
 		return ret;
 
-	fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
-	if (fd < 0)
-		return -errno;
-
 	for (i = 0; i < nblocks; ++i) {
 		char buf[EROFS_BLKSIZ];
 
 		ret = read(fd, buf, EROFS_BLKSIZ);
 		if (ret != EROFS_BLKSIZ) {
 			if (ret < 0)
-				goto fail;
-			close(fd);
+				return -errno;
 			return -EAGAIN;
 		}
 
 		ret = blk_write(buf, inode->u.i_blkaddr + i, 1);
 		if (ret)
-			goto fail;
+			return ret;
 	}
 
 	/* read the tail-end data */
 	inode->idata_size = inode->i_size % EROFS_BLKSIZ;
 	if (inode->idata_size) {
 		inode->idata = malloc(inode->idata_size);
-		if (!inode->idata) {
-			close(fd);
+		if (!inode->idata)
 			return -ENOMEM;
-		}
 
 		ret = read(fd, inode->idata, inode->idata_size);
 		if (ret < inode->idata_size) {
 			free(inode->idata);
 			inode->idata = NULL;
-			close(fd);
 			return -EIO;
 		}
 	}
-	close(fd);
 	return 0;
-fail:
-	ret = -errno;
+}
+
+int erofs_write_file(struct erofs_inode *inode)
+{
+	int ret, fd;
+
+	if (!inode->i_size) {
+		inode->datalayout = EROFS_INODE_FLAT_PLAIN;
+		return 0;
+	}
+
+	if (cfg.c_compr_alg_master && erofs_file_is_compressible(inode)) {
+		ret = erofs_write_compressed_file(inode);
+
+		if (!ret || ret != -ENOSPC)
+			return ret;
+	}
+
+	/* fallback to all data uncompressed */
+	fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
+	if (fd < 0)
+		return -errno;
+
+	ret = write_uncompressed_file_from_fd(inode, fd);
 	close(fd);
 	return ret;
 }
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-12-19 18:19 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-18 15:52 [PATCH v1] erofs-utils:code clean of write file Li Guifu
2019-12-19 18:19 ` Gao Xiang via Linux-erofs

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.