* [PATCH 1/2] sparse_image_fd: add function to re-use a file descriptor @ 2023-09-14 9:58 Juergen Borleis 2023-09-14 9:58 ` [PATCH 2/2] uncompress: add Android sparse image support Juergen Borleis 0 siblings, 1 reply; 5+ messages in thread From: Juergen Borleis @ 2023-09-14 9:58 UTC (permalink / raw) To: barebox Signed-off-by: Juergen Borleis <jbe@pengutronix.de> --- include/image-sparse.h | 1 + lib/image-sparse.c | 17 +++++++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/include/image-sparse.h b/include/image-sparse.h index 6bff844..d969ca1 100644 --- a/include/image-sparse.h +++ b/include/image-sparse.h @@ -59,6 +59,7 @@ static inline int is_sparse_image(const void *buf) struct sparse_image_ctx; struct sparse_image_ctx *sparse_image_open(const char *path); +struct sparse_image_ctx *sparse_image_fd(int infd); int sparse_image_read(struct sparse_image_ctx *si, void *buf, loff_t *pos, size_t len, size_t *retlen); void sparse_image_close(struct sparse_image_ctx *si); diff --git a/lib/image-sparse.c b/lib/image-sparse.c index eb5242e..b686d25 100644 --- a/lib/image-sparse.c +++ b/lib/image-sparse.c @@ -146,7 +146,7 @@ loff_t sparse_image_size(struct sparse_image_ctx *si) return (loff_t)si->sparse.blk_sz * si->sparse.total_blks; } -struct sparse_image_ctx *sparse_image_open(const char *path) +struct sparse_image_ctx *sparse_image_fd(int infd) { struct sparse_image_ctx *si; loff_t offs; @@ -154,11 +154,7 @@ struct sparse_image_ctx *sparse_image_open(const char *path) si = xzalloc(sizeof(*si)); - si->fd = open(path, O_RDONLY); - if (si->fd < 0) { - ret = -errno; - goto out; - } + si->fd = infd; /* Read and skip over sparse image header */ read(si->fd, &si->sparse, sizeof(struct sparse_header)); @@ -186,6 +182,15 @@ struct sparse_image_ctx *sparse_image_open(const char *path) return ERR_PTR(ret); } +struct sparse_image_ctx *sparse_image_open(const char *path) +{ + int fd = open(path, O_RDONLY); + if (fd < 0) + return ERR_PTR(-errno); + + return sparse_image_fd(fd); +} + int sparse_image_read(struct sparse_image_ctx *si, void *buf, loff_t *pos, size_t len, size_t *retlen) { -- 2.30.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] uncompress: add Android sparse image support 2023-09-14 9:58 [PATCH 1/2] sparse_image_fd: add function to re-use a file descriptor Juergen Borleis @ 2023-09-14 9:58 ` Juergen Borleis 2023-09-16 16:01 ` Uwe Kleine-König 2023-09-21 12:01 ` Sascha Hauer 0 siblings, 2 replies; 5+ messages in thread From: Juergen Borleis @ 2023-09-14 9:58 UTC (permalink / raw) To: barebox An Android sparse image file generated by the 'genimage' tool can be seen as a kind of compression, since it contains only the data which should really be written to a device. This implementation writes only the intended content and skips anything else. Signed-off-by: Juergen Borleis <jbe@pengutronix.de> --- lib/uncompress.c | 82 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 78 insertions(+), 4 deletions(-) diff --git a/lib/uncompress.c b/lib/uncompress.c index 0608e9f..f17e9ce 100644 --- a/lib/uncompress.c +++ b/lib/uncompress.c @@ -25,6 +25,10 @@ #include <malloc.h> #include <fs.h> #include <libfile.h> +#ifdef CONFIG_IMAGE_SPARSE +#include <image-sparse.h> +#include <linux/sizes.h> +#endif static void *uncompress_buf; static unsigned int uncompress_size; @@ -60,6 +64,71 @@ static int uncompress_fill(void *buf, unsigned int len) return total; } +#ifdef CONFIG_IMAGE_SPARSE +static int uncompress_sparse(int source, int destination) +{ + int ret = 0; + struct sparse_image_ctx *sparse; + + /* rewind after checking the file type */ + lseek(source, 0, SEEK_SET); + + sparse = sparse_image_fd(source); + if (IS_ERR(sparse)) { + pr_err("Failed to open or interpret sparse image file: %m\n"); + ret = 1; + goto on_error_dev; + } + + const size_t transfer_buffer_length = SZ_128K; + unsigned char *buf = malloc(transfer_buffer_length); + if (!buf) { + pr_err("Failed to alloc memory for the transfers\n"); + ret = 1; + goto on_error_sparse; + } + + while (1) { + loff_t write_offset; + size_t write_length; + + int rc = sparse_image_read(sparse, buf, &write_offset, transfer_buffer_length, &write_length); + if (rc) { + ret = 1; + goto on_error_memory; + } + if (!write_length) + break; + + discard_range(destination, write_length, write_offset); + + write_offset = lseek(destination, write_offset, SEEK_SET); + if (write_offset == -1) { + pr_err("Failed to set next data's destination offset: %m\n"); + ret = 1; + goto on_error_memory; + + } + + rc = write_full(destination, buf, write_length); + if (rc < 0) { + pr_err("Failed to write destination's next data: %m\n"); + ret = 1; + goto on_error_memory; + } + } + +on_error_memory: + free(buf); +on_error_sparse: + free(sparse); /* Note: sparse_image_close(sparse); would also close the input file descriptor */ +on_error_dev: + return ret; +} +#endif + +static int uncompress_infd, uncompress_outfd; + int uncompress(unsigned char *inbuf, int len, int(*fill)(void*, unsigned int), int(*flush)(void*, unsigned int), @@ -121,6 +190,10 @@ int uncompress(unsigned char *inbuf, int len, case filetype_xz_compressed: compfn = decompress_unxz; break; +#endif +#ifdef CONFIG_IMAGE_SPARSE + case filetype_android_sparse: + break; #endif default: err = basprintf("cannot handle filetype %s", @@ -131,16 +204,17 @@ int uncompress(unsigned char *inbuf, int len, goto err; } - ret = compfn(inbuf, len, fill ? uncompress_fill : NULL, - flush, output, pos, error_fn); + if (ft == filetype_android_sparse) + ret = uncompress_sparse(uncompress_infd, uncompress_outfd); + else + ret = compfn(inbuf, len, fill ? uncompress_fill : NULL, + flush, output, pos, error_fn); err: free(uncompress_buf); return ret; } -static int uncompress_infd, uncompress_outfd; - static int fill_fd(void *buf, unsigned int len) { return read_full(uncompress_infd, buf, len); -- 2.30.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] uncompress: add Android sparse image support 2023-09-14 9:58 ` [PATCH 2/2] uncompress: add Android sparse image support Juergen Borleis @ 2023-09-16 16:01 ` Uwe Kleine-König 2023-09-21 12:04 ` Sascha Hauer 2023-09-21 12:01 ` Sascha Hauer 1 sibling, 1 reply; 5+ messages in thread From: Uwe Kleine-König @ 2023-09-16 16:01 UTC (permalink / raw) To: Juergen Borleis; +Cc: barebox [-- Attachment #1: Type: text/plain, Size: 4252 bytes --] On Thu, Sep 14, 2023 at 11:58:59AM +0200, Juergen Borleis wrote: > An Android sparse image file generated by the 'genimage' tool can be seen as a > kind of compression, since it contains only the data which should really be > written to a device. > > This implementation writes only the intended content and skips anything else. > > Signed-off-by: Juergen Borleis <jbe@pengutronix.de> Tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> while this obviously works, I still have a comment below: > --- > lib/uncompress.c | 82 +++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 78 insertions(+), 4 deletions(-) > > diff --git a/lib/uncompress.c b/lib/uncompress.c > index 0608e9f..f17e9ce 100644 > --- a/lib/uncompress.c > +++ b/lib/uncompress.c > @@ -25,6 +25,10 @@ > #include <malloc.h> > #include <fs.h> > #include <libfile.h> > +#ifdef CONFIG_IMAGE_SPARSE > +#include <image-sparse.h> > +#include <linux/sizes.h> > +#endif > > static void *uncompress_buf; > static unsigned int uncompress_size; > @@ -60,6 +64,71 @@ static int uncompress_fill(void *buf, unsigned int len) > return total; > } > > +#ifdef CONFIG_IMAGE_SPARSE > +static int uncompress_sparse(int source, int destination) > +{ > + int ret = 0; > + struct sparse_image_ctx *sparse; > + > + /* rewind after checking the file type */ > + lseek(source, 0, SEEK_SET); > + > + sparse = sparse_image_fd(source); > + if (IS_ERR(sparse)) { > + pr_err("Failed to open or interpret sparse image file: %m\n"); > + ret = 1; > + goto on_error_dev; > + } > + > + const size_t transfer_buffer_length = SZ_128K; > + unsigned char *buf = malloc(transfer_buffer_length); > + if (!buf) { > + pr_err("Failed to alloc memory for the transfers\n"); > + ret = 1; > + goto on_error_sparse; > + } > + > + while (1) { > + loff_t write_offset; > + size_t write_length; > + > + int rc = sparse_image_read(sparse, buf, &write_offset, transfer_buffer_length, &write_length); > + if (rc) { > + ret = 1; > + goto on_error_memory; > + } > + if (!write_length) > + break; > + > + discard_range(destination, write_length, write_offset); > + > + write_offset = lseek(destination, write_offset, SEEK_SET); > + if (write_offset == -1) { > + pr_err("Failed to set next data's destination offset: %m\n"); > + ret = 1; > + goto on_error_memory; > + > + } > + > + rc = write_full(destination, buf, write_length); > + if (rc < 0) { > + pr_err("Failed to write destination's next data: %m\n"); > + ret = 1; > + goto on_error_memory; > + } > + } > + > +on_error_memory: > + free(buf); > +on_error_sparse: > + free(sparse); /* Note: sparse_image_close(sparse); would also close the input file descriptor */ > +on_error_dev: > + return ret; > +} > +#endif > + > +static int uncompress_infd, uncompress_outfd; > + > int uncompress(unsigned char *inbuf, int len, > int(*fill)(void*, unsigned int), > int(*flush)(void*, unsigned int), > @@ -121,6 +190,10 @@ int uncompress(unsigned char *inbuf, int len, > case filetype_xz_compressed: > compfn = decompress_unxz; > break; > +#endif > +#ifdef CONFIG_IMAGE_SPARSE > + case filetype_android_sparse: > + break; > #endif > default: > err = basprintf("cannot handle filetype %s", > @@ -131,16 +204,17 @@ int uncompress(unsigned char *inbuf, int len, > goto err; > } > > - ret = compfn(inbuf, len, fill ? uncompress_fill : NULL, > - flush, output, pos, error_fn); > + if (ft == filetype_android_sparse) > + ret = uncompress_sparse(uncompress_infd, uncompress_outfd); > + else > + ret = compfn(inbuf, len, fill ? uncompress_fill : NULL, > + flush, output, pos, error_fn); Wouldn't it be more natural to make uncompress_sparse use the same prototype as the other uncompress functions and then just have: #ifdef CONFIG_IMAGE_SPARSE case filetype_android_sparse: compfn = decompress_sparse; break; #endif Without the need to adapt the compfn call? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] uncompress: add Android sparse image support 2023-09-16 16:01 ` Uwe Kleine-König @ 2023-09-21 12:04 ` Sascha Hauer 0 siblings, 0 replies; 5+ messages in thread From: Sascha Hauer @ 2023-09-21 12:04 UTC (permalink / raw) To: Uwe Kleine-König; +Cc: Juergen Borleis, barebox On Sat, Sep 16, 2023 at 06:01:15PM +0200, Uwe Kleine-König wrote: > On Thu, Sep 14, 2023 at 11:58:59AM +0200, Juergen Borleis wrote: > > An Android sparse image file generated by the 'genimage' tool can be seen as a > > kind of compression, since it contains only the data which should really be > > written to a device. > > > > This implementation writes only the intended content and skips anything else. > > > > Signed-off-by: Juergen Borleis <jbe@pengutronix.de> > > Tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > while this obviously works, I still have a comment below: > > > --- > > lib/uncompress.c | 82 +++++++++++++++++++++++++++++++++++++++++++++--- > > 1 file changed, 78 insertions(+), 4 deletions(-) > > > > diff --git a/lib/uncompress.c b/lib/uncompress.c > > index 0608e9f..f17e9ce 100644 > > --- a/lib/uncompress.c > > +++ b/lib/uncompress.c > > @@ -25,6 +25,10 @@ > > #include <malloc.h> > > #include <fs.h> > > #include <libfile.h> > > +#ifdef CONFIG_IMAGE_SPARSE > > +#include <image-sparse.h> > > +#include <linux/sizes.h> > > +#endif > > > > static void *uncompress_buf; > > static unsigned int uncompress_size; > > @@ -60,6 +64,71 @@ static int uncompress_fill(void *buf, unsigned int len) > > return total; > > } > > > > +#ifdef CONFIG_IMAGE_SPARSE > > +static int uncompress_sparse(int source, int destination) > > +{ > > + int ret = 0; > > + struct sparse_image_ctx *sparse; > > + > > + /* rewind after checking the file type */ > > + lseek(source, 0, SEEK_SET); > > + > > + sparse = sparse_image_fd(source); > > + if (IS_ERR(sparse)) { > > + pr_err("Failed to open or interpret sparse image file: %m\n"); > > + ret = 1; > > + goto on_error_dev; > > + } > > + > > + const size_t transfer_buffer_length = SZ_128K; > > + unsigned char *buf = malloc(transfer_buffer_length); > > + if (!buf) { > > + pr_err("Failed to alloc memory for the transfers\n"); > > + ret = 1; > > + goto on_error_sparse; > > + } > > + > > + while (1) { > > + loff_t write_offset; > > + size_t write_length; > > + > > + int rc = sparse_image_read(sparse, buf, &write_offset, transfer_buffer_length, &write_length); > > + if (rc) { > > + ret = 1; > > + goto on_error_memory; > > + } > > + if (!write_length) > > + break; > > + > > + discard_range(destination, write_length, write_offset); > > + > > + write_offset = lseek(destination, write_offset, SEEK_SET); > > + if (write_offset == -1) { > > + pr_err("Failed to set next data's destination offset: %m\n"); > > + ret = 1; > > + goto on_error_memory; > > + > > + } > > + > > + rc = write_full(destination, buf, write_length); > > + if (rc < 0) { > > + pr_err("Failed to write destination's next data: %m\n"); > > + ret = 1; > > + goto on_error_memory; > > + } > > + } > > + > > +on_error_memory: > > + free(buf); > > +on_error_sparse: > > + free(sparse); /* Note: sparse_image_close(sparse); would also close the input file descriptor */ > > +on_error_dev: > > + return ret; > > +} > > +#endif > > + > > +static int uncompress_infd, uncompress_outfd; > > + > > int uncompress(unsigned char *inbuf, int len, > > int(*fill)(void*, unsigned int), > > int(*flush)(void*, unsigned int), > > @@ -121,6 +190,10 @@ int uncompress(unsigned char *inbuf, int len, > > case filetype_xz_compressed: > > compfn = decompress_unxz; > > break; > > +#endif > > +#ifdef CONFIG_IMAGE_SPARSE > > + case filetype_android_sparse: > > + break; > > #endif > > default: > > err = basprintf("cannot handle filetype %s", > > @@ -131,16 +204,17 @@ int uncompress(unsigned char *inbuf, int len, > > goto err; > > } > > > > - ret = compfn(inbuf, len, fill ? uncompress_fill : NULL, > > - flush, output, pos, error_fn); > > + if (ft == filetype_android_sparse) > > + ret = uncompress_sparse(uncompress_infd, uncompress_outfd); > > + else > > + ret = compfn(inbuf, len, fill ? uncompress_fill : NULL, > > + flush, output, pos, error_fn); > > Wouldn't it be more natural to make uncompress_sparse use the same > prototype as the other uncompress functions and then just have: > > #ifdef CONFIG_IMAGE_SPARSE > case filetype_android_sparse: > compfn = decompress_sparse; > break; > #endif > > Without the need to adapt the compfn call? compfn() doesn't have access to the file descriptors which the current sparse implementation needs. It would be great though to let our sparse image support use an input buffer and a fill() function. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] uncompress: add Android sparse image support 2023-09-14 9:58 ` [PATCH 2/2] uncompress: add Android sparse image support Juergen Borleis 2023-09-16 16:01 ` Uwe Kleine-König @ 2023-09-21 12:01 ` Sascha Hauer 1 sibling, 0 replies; 5+ messages in thread From: Sascha Hauer @ 2023-09-21 12:01 UTC (permalink / raw) To: Juergen Borleis; +Cc: barebox On Thu, Sep 14, 2023 at 11:58:59AM +0200, Juergen Borleis wrote: > An Android sparse image file generated by the 'genimage' tool can be seen as a > kind of compression, since it contains only the data which should really be > written to a device. > > This implementation writes only the intended content and skips anything else. > > Signed-off-by: Juergen Borleis <jbe@pengutronix.de> > --- > lib/uncompress.c | 82 +++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 78 insertions(+), 4 deletions(-) > > diff --git a/lib/uncompress.c b/lib/uncompress.c > index 0608e9f..f17e9ce 100644 > --- a/lib/uncompress.c > +++ b/lib/uncompress.c > @@ -25,6 +25,10 @@ > #include <malloc.h> > #include <fs.h> > #include <libfile.h> > +#ifdef CONFIG_IMAGE_SPARSE > +#include <image-sparse.h> > +#include <linux/sizes.h> > +#endif > > static void *uncompress_buf; > static unsigned int uncompress_size; > @@ -60,6 +64,71 @@ static int uncompress_fill(void *buf, unsigned int len) > return total; > } > > +#ifdef CONFIG_IMAGE_SPARSE > +static int uncompress_sparse(int source, int destination) > +{ > + int ret = 0; > + struct sparse_image_ctx *sparse; > + You could drop all the #ifdefs and add a if (!IS_ENABLED(CONFIG_IMAGE_SPARSE)) { pr_err("Sparse image support disabled\n"); return -ENOSYS; } instead. > + /* rewind after checking the file type */ > + lseek(source, 0, SEEK_SET); Why is this necessary? If this is necessary for sparse images it should also be necessary for other compression types as well and should be done by the caller. > + > + sparse = sparse_image_fd(source); > + if (IS_ERR(sparse)) { > + pr_err("Failed to open or interpret sparse image file: %m\n"); > + ret = 1; '1' is not a valid error code. PTR_ERR(sparse) would be suitable here. > + goto on_error_dev; > + } > + > + const size_t transfer_buffer_length = SZ_128K; > + unsigned char *buf = malloc(transfer_buffer_length); > + if (!buf) { > + pr_err("Failed to alloc memory for the transfers\n"); No need to print. just return -ENOMEM. Please return valid error codes in the rest of this function as well. > + ret = 1; > + goto on_error_sparse; > + } > + > + while (1) { > + loff_t write_offset; > + size_t write_length; > + > + int rc = sparse_image_read(sparse, buf, &write_offset, transfer_buffer_length, &write_length); > + if (rc) { > + ret = 1; > + goto on_error_memory; > + } > + if (!write_length) > + break; > + > + discard_range(destination, write_length, write_offset); > + > + write_offset = lseek(destination, write_offset, SEEK_SET); > + if (write_offset == -1) { > + pr_err("Failed to set next data's destination offset: %m\n"); > + ret = 1; > + goto on_error_memory; > + > + } > + > + rc = write_full(destination, buf, write_length); > + if (rc < 0) { > + pr_err("Failed to write destination's next data: %m\n"); > + ret = 1; > + goto on_error_memory; > + } > + } > + > +on_error_memory: > + free(buf); > +on_error_sparse: > + free(sparse); /* Note: sparse_image_close(sparse); would also close the input file descriptor */ > +on_error_dev: > + return ret; > +} > +#endif > + > +static int uncompress_infd, uncompress_outfd; > + > int uncompress(unsigned char *inbuf, int len, > int(*fill)(void*, unsigned int), > int(*flush)(void*, unsigned int), > @@ -121,6 +190,10 @@ int uncompress(unsigned char *inbuf, int len, > case filetype_xz_compressed: > compfn = decompress_unxz; > break; > +#endif > +#ifdef CONFIG_IMAGE_SPARSE > + case filetype_android_sparse: > + break; > #endif > default: > err = basprintf("cannot handle filetype %s", > @@ -131,16 +204,17 @@ int uncompress(unsigned char *inbuf, int len, > goto err; > } > > - ret = compfn(inbuf, len, fill ? uncompress_fill : NULL, > - flush, output, pos, error_fn); > + if (ft == filetype_android_sparse) > + ret = uncompress_sparse(uncompress_infd, uncompress_outfd); This assumes both uncompress_infd and uncompress_outfd are valid which is the case for uncompress_fd_to_fd() only, but not for uncompress_buf_to_fd() or uncompress_buf_to_buf(). I think this limitation is okay for sparse image support, but the other cases should return an error instead of silently going crazy on invalid file descriptors. BTW at least sparse image support for uncompress_fd_to_buf() could be implemented without much hassle. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-09-21 12:05 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-09-14 9:58 [PATCH 1/2] sparse_image_fd: add function to re-use a file descriptor Juergen Borleis 2023-09-14 9:58 ` [PATCH 2/2] uncompress: add Android sparse image support Juergen Borleis 2023-09-16 16:01 ` Uwe Kleine-König 2023-09-21 12:04 ` Sascha Hauer 2023-09-21 12:01 ` Sascha Hauer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox