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=vwpxf2xfxcw8r67sb3kv1phrn2zmg23...@mail.gmail.com/ On Wed, Dec 18, 2019 at 11:52:37PM +0800, Li Guifu wrote: > From: Li Guifu <[email protected]> > > Make a code clean at function erofs_write_file() which > has multi jump. I've rewriten the commit message. > > Signed-off-by: Li Guifu <[email protected]> > --- > 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 <[email protected]> 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 <[email protected]> Signed-off-by: Gao Xiang <[email protected]> --- 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
