From mboxrd@z Thu Jan 1 00:00:00 1970 Delivery-date: Thu, 21 Sep 2023 14:03:42 +0200 Received: from metis.whiteo.stw.pengutronix.de ([2a0a:edc0:2:b01:1d::104]) by lore.white.stw.pengutronix.de with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1qjIPH-005qfZ-G4 for lore@lore.pengutronix.de; Thu, 21 Sep 2023 14:03:42 +0200 Received: from bombadil.infradead.org ([2607:7c80:54:3::133]) by metis.whiteo.stw.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1qjIPF-000271-Ho for lore@pengutronix.de; Thu, 21 Sep 2023 14:03:42 +0200 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:From:In-Reply-To: Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=lXalhT8fBORLyRk1/dvknytbXZF+RKzlx9YkE9rtznk=; b=rmBCQoZiXH60cbkqiGN6bDrs5a 2mywS4VfvkC201jAkCQl29xZk+8kTY2xfasP7ZS9L7suLX9UINtQyOantP/o98P6EO6xsTIuc3E/U MWiaSaGAiXOLfwwSHgr8Khb8F6TgmKLTO9U+62hP/hlPYe1dtUVWranflFPuxxrWGstG6u4N5VmFa xh8IPpx+u2/MAy5T83pQRW32z68VPAUWOWgbZ2cekfXdzcoSoOWtvzjpB4Dnl2AmaYEWdpPY5X3Fz g3BZuCD4lee3Dg+SSHvRPzaD85bpuAaJ+tqbqj0KSDroxLv5PpDZ85T0TPKx/RJ1lqejnGUYbJeXi U5rBXE4g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qjINn-005xMw-0s; Thu, 21 Sep 2023 12:02:11 +0000 Received: from metis.whiteo.stw.pengutronix.de ([2a0a:edc0:2:b01:1d::104]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qjINc-005xLp-0C for barebox@lists.infradead.org; Thu, 21 Sep 2023 12:02:04 +0000 Received: from drehscheibe.grey.stw.pengutronix.de ([2a0a:edc0:0:c01:1d::a2]) by metis.whiteo.stw.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1qjINa-0001NL-Mo; Thu, 21 Sep 2023 14:01:58 +0200 Received: from [2a0a:edc0:2:b01:1d::c0] (helo=ptx.whiteo.stw.pengutronix.de) by drehscheibe.grey.stw.pengutronix.de with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1qjINa-007unc-AL; Thu, 21 Sep 2023 14:01:58 +0200 Received: from sha by ptx.whiteo.stw.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1qjINa-00Akol-7m; Thu, 21 Sep 2023 14:01:58 +0200 Date: Thu, 21 Sep 2023 14:01:58 +0200 To: Juergen Borleis Cc: barebox@lists.infradead.org Message-ID: <20230921120158.GI637806@pengutronix.de> References: <20230914095859.22166-1-jbe@pengutronix.de> <20230914095859.22166-2-jbe@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230914095859.22166-2-jbe@pengutronix.de> X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-Accept-Language: de,en X-Accept-Content-Type: text/plain User-Agent: Mutt/1.10.1 (2018-07-13) From: Sascha Hauer X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230921_050200_106257_AB72C46A X-CRM114-Status: GOOD ( 36.01 ) X-BeenThere: barebox@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "barebox" X-SA-Exim-Connect-IP: 2607:7c80:54:3::133 X-SA-Exim-Mail-From: barebox-bounces+lore=pengutronix.de@lists.infradead.org X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on metis.whiteo.stw.pengutronix.de X-Spam-Level: X-Spam-Status: No, score=-5.0 required=4.0 tests=AWL,BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE autolearn=unavailable autolearn_force=no version=3.4.2 Subject: Re: [PATCH 2/2] uncompress: add Android sparse image support X-SA-Exim-Version: 4.2.1 (built Wed, 08 May 2019 21:11:16 +0000) X-SA-Exim-Scanned: Yes (on metis.whiteo.stw.pengutronix.de) 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 > --- > 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 > #include > #include > +#ifdef CONFIG_IMAGE_SPARSE > +#include > +#include > +#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 |