diff mbox series

[3/4] copyfile cleanup: use copyimage when we can

Message ID 20240617072905.2275940-4-dominique.martinet@atmark-techno.com
State Accepted
Headers show
Series misc fixes from chunked stream rework | expand

Commit Message

Dominique Martinet June 17, 2024, 7:29 a.m. UTC
There were a few handlers manually passing all the fields required for copyimage,
use copyimage instead.

lua was using img.checksum as a secondary hash, preserve this
functionality by remembering the old img.checksum instead of passing a
different checksum pointer.

There is no intended functional change.

Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
---
 corelib/lua_interface.c  | 38 ++++++++------------------------------
 handlers/delta_handler.c | 13 +------------
 handlers/rdiff_handler.c | 13 +------------
 3 files changed, 10 insertions(+), 54 deletions(-)

Comments

Michael Glembotzki June 18, 2024, 7:45 p.m. UTC | #1
Reviewed-by: Michael Glembotzki <Michael.Glembotzki@iris-sensing.com>

Dominique Martinet schrieb am Montag, 17. Juni 2024 um 09:29:15 UTC+2:

> There were a few handlers manually passing all the fields required for 
> copyimage,
> use copyimage instead.
>
> lua was using img.checksum as a secondary hash, preserve this
> functionality by remembering the old img.checksum instead of passing a
> different checksum pointer.
>
> There is no intended functional change.
>
> Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
> ---
> corelib/lua_interface.c | 38 ++++++++------------------------------
> handlers/delta_handler.c | 13 +------------
> handlers/rdiff_handler.c | 13 +------------
> 3 files changed, 10 insertions(+), 54 deletions(-)
>
> diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c
> index 03b74aed1cf2..10af0760c5f9 100644
> --- a/corelib/lua_interface.c
> +++ b/corelib/lua_interface.c
> @@ -388,7 +388,7 @@ static int l_copy2file(lua_State *L)
> }
>
> struct img_type img = {};
> - uint32_t checksum = 0;
> + uint32_t image_checksum = img.checksum;
>
> table2image(L, &img);
> if (check_same_file(img.fdin, fdout)) {
> @@ -397,18 +397,7 @@ static int l_copy2file(lua_State *L)
> lua_pushstring(L, "Output file path is same as input file temporary path");
> goto copyfile_exit;
> }
> - int ret = copyfile(img.fdin,
> - &fdout,
> - img.size,
> - (unsigned long *)&img.offset,
> - img.seek,
> - 0, /* no skip */
> - img.compressed,
> - &checksum,
> - img.sha256,
> - img.is_encrypted,
> - img.ivt_ascii,
> - NULL);
> + int ret = copyimage(&fdout, &img, NULL /* default write callback */);
> update_table(L, &img);
> lua_pop(L, 1);
>
> @@ -417,10 +406,10 @@ static int l_copy2file(lua_State *L)
> lua_pushstring(L, strerror(errno));
> goto copyfile_exit;
> }
> - if ((img.checksum != 0) && (checksum != img.checksum)) {
> + if ((image_checksum != 0) && (image_checksum != img.checksum)) {
> lua_pushinteger(L, -1);
> lua_pushfstring(L, "Checksums WRONG! Computed 0x%d, should be 0x%d\n",
> - checksum, img.checksum);
> + img.checksum, image_checksum);
> goto copyfile_exit;
> }
>
> @@ -463,24 +452,13 @@ static int l_istream_read(lua_State* L)
> luaL_checktype(L, 2, LUA_TFUNCTION);
>
> struct img_type img = {};
> - uint32_t checksum = 0;
> + uint32_t image_checksum = img.checksum;
>
> lua_pushvalue(L, 1);
> table2image(L, &img);
> lua_pop(L, 1);
>
> - int ret = copyfile(img.fdin,
> - L,
> - img.size,
> - (unsigned long *)&img.offset,
> - img.seek,
> - 0, /* no skip */
> - img.compressed,
> - &checksum,
> - img.sha256,
> - img.is_encrypted,
> - img.ivt_ascii,
> - istream_read_callback);
> + int ret = copyimage(L, &img, istream_read_callback);
>
> lua_pop(L, 1);
> update_table(L, &img);
> @@ -491,10 +469,10 @@ static int l_istream_read(lua_State* L)
> lua_pushstring(L, strerror(errno));
> return 2;
> }
> - if ((img.checksum != 0) && (checksum != img.checksum)) {
> + if ((image_checksum != 0) && (image_checksum != img.checksum)) {
> lua_pushinteger(L, -1);
> lua_pushfstring(L, "Checksums WRONG! Computed 0x%d, should be 0x%d\n",
> - checksum, img.checksum);
> + img.checksum, image_checksum);
> return 2;
> }
> lua_pushinteger(L, 0);
> diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c
> index 8e7faa0ead02..5ec243ee01d9 100644
> --- a/handlers/delta_handler.c
> +++ b/handlers/delta_handler.c
> @@ -999,18 +999,7 @@ static int install_delta(struct img_type *img,
> goto cleanup;
> }
>
> - ret = copyfile(img->fdin,
> - &mem_fd,
> - img->size,
> - (unsigned long *)&img->offset,
> - img->seek,
> - 0,
> - img->compressed,
> - &img->checksum,
> - img->sha256,
> - img->is_encrypted,
> - img->ivt_ascii,
> - NULL);
> + ret = copyimage(&mem_fd, img, NULL /* default write callback */);
>
> if (ret != 0) {
> ERROR("Error %d copying zchunk header, aborting.", ret);
> diff --git a/handlers/rdiff_handler.c b/handlers/rdiff_handler.c
> index e01a127bce50..3b01519ce80a 100644
> --- a/handlers/rdiff_handler.c
> +++ b/handlers/rdiff_handler.c
> @@ -337,18 +337,7 @@ static int apply_rdiff_patch(struct img_type *img,
> rs_trace_to(rdiff_log);
>
> rdiff_state.job = rs_patch_begin(base_file_read_cb, rdiff_state.base_file);
> - ret = copyfile(img->fdin,
> - &rdiff_state,
> - img->size,
> - (unsigned long *)&img->offset,
> - img->seek,
> - 0, /* no skip */
> - img->compressed,
> - &img->checksum,
> - img->sha256,
> - img->is_encrypted,
> - img->ivt_ascii,
> - apply_rdiff_chunk_cb);
> + ret = copyimage(&rdiff_state, img, apply_rdiff_chunk_cb);
> if (ret != 0) {
> ERROR("Error %d running rdiff job, aborting.", ret);
> goto cleanup;
> -- 
> 2.39.2
>
>
>
Stefano Babic June 18, 2024, 8:59 p.m. UTC | #2
Hi Dominique,

On 17.06.24 09:29, Dominique Martinet wrote:
> There were a few handlers manually passing all the fields required for copyimage,
> use copyimage instead.
>
> lua was using img.checksum as a secondary hash, preserve this
> functionality by remembering the old img.checksum instead of passing a
> different checksum pointer.
>

Ouch, I forgot to switch from copyfile to copyimage for the Lua code.
Thanks for this !

> There is no intended functional change.
>
> Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
> ---
>   corelib/lua_interface.c  | 38 ++++++++------------------------------
>   handlers/delta_handler.c | 13 +------------
>   handlers/rdiff_handler.c | 13 +------------
>   3 files changed, 10 insertions(+), 54 deletions(-)
>
> diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c
> index 03b74aed1cf2..10af0760c5f9 100644
> --- a/corelib/lua_interface.c
> +++ b/corelib/lua_interface.c
> @@ -388,7 +388,7 @@ static int l_copy2file(lua_State *L)
>   	}
>
>   	struct img_type img = {};
> -	uint32_t checksum = 0;
> +	uint32_t image_checksum = img.checksum;
>
>   	table2image(L, &img);
>   	if (check_same_file(img.fdin, fdout)) {
> @@ -397,18 +397,7 @@ static int l_copy2file(lua_State *L)
>   		lua_pushstring(L, "Output file path is same as input file temporary path");
>   		goto copyfile_exit;
>   	}
> -	int ret = copyfile(img.fdin,
> -				 &fdout,
> -				 img.size,
> -				 (unsigned long *)&img.offset,
> -				 img.seek,
> -				 0, /* no skip */
> -				 img.compressed,
> -				 &checksum,
> -				 img.sha256,
> -				 img.is_encrypted,
> -				 img.ivt_ascii,
> -				 NULL);
> +	int ret = copyimage(&fdout, &img, NULL /* default write callback */);
>   	update_table(L, &img);
>   	lua_pop(L, 1);
>
> @@ -417,10 +406,10 @@ static int l_copy2file(lua_State *L)
>   		lua_pushstring(L, strerror(errno));
>   		goto copyfile_exit;
>   	}
> -	if ((img.checksum != 0) && (checksum != img.checksum)) {
> +	if ((image_checksum != 0) && (image_checksum != img.checksum)) {
>   		lua_pushinteger(L, -1);
>   		lua_pushfstring(L, "Checksums WRONG! Computed 0x%d, should be 0x%d\n",
> -						checksum, img.checksum);
> +				img.checksum, image_checksum);
>   		goto copyfile_exit;
>   	}
>
> @@ -463,24 +452,13 @@ static int l_istream_read(lua_State* L)
>   	luaL_checktype(L, 2, LUA_TFUNCTION);
>
>   	struct img_type img = {};
> -	uint32_t checksum = 0;
> +	uint32_t image_checksum = img.checksum;
>
>   	lua_pushvalue(L, 1);
>   	table2image(L, &img);
>   	lua_pop(L, 1);
>
> -	int ret = copyfile(img.fdin,
> -				 L,
> -				 img.size,
> -				 (unsigned long *)&img.offset,
> -				 img.seek,
> -				 0, /* no skip */
> -				 img.compressed,
> -				 &checksum,
> -				 img.sha256,
> -				 img.is_encrypted,
> -				 img.ivt_ascii,
> -				 istream_read_callback);
> +	int ret = copyimage(L, &img, istream_read_callback);
>
>   	lua_pop(L, 1);
>   	update_table(L, &img);
> @@ -491,10 +469,10 @@ static int l_istream_read(lua_State* L)
>   		lua_pushstring(L, strerror(errno));
>   		return 2;
>   	}
> -	if ((img.checksum != 0) && (checksum != img.checksum)) {
> +	if ((image_checksum != 0) && (image_checksum != img.checksum)) {
>   		lua_pushinteger(L, -1);
>   		lua_pushfstring(L, "Checksums WRONG! Computed 0x%d, should be 0x%d\n",
> -						checksum, img.checksum);
> +				img.checksum, image_checksum);
>   		return 2;
>   	}
>   	lua_pushinteger(L, 0);
> diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c
> index 8e7faa0ead02..5ec243ee01d9 100644
> --- a/handlers/delta_handler.c
> +++ b/handlers/delta_handler.c
> @@ -999,18 +999,7 @@ static int install_delta(struct img_type *img,
>   		goto cleanup;
>   	}
>
> -	ret = copyfile(img->fdin,
> -		&mem_fd,
> -		img->size,
> -		(unsigned long *)&img->offset,
> -		img->seek,
> -		0,
> -		img->compressed,
> -		&img->checksum,
> -		img->sha256,
> -		img->is_encrypted,
> -		img->ivt_ascii,
> -		NULL);
> +	ret = copyimage(&mem_fd, img, NULL /* default write callback */);
>
>   	if (ret != 0) {
>   		ERROR("Error %d copying zchunk header, aborting.", ret);
> diff --git a/handlers/rdiff_handler.c b/handlers/rdiff_handler.c
> index e01a127bce50..3b01519ce80a 100644
> --- a/handlers/rdiff_handler.c
> +++ b/handlers/rdiff_handler.c
> @@ -337,18 +337,7 @@ static int apply_rdiff_patch(struct img_type *img,
>   	rs_trace_to(rdiff_log);
>
>   	rdiff_state.job = rs_patch_begin(base_file_read_cb, rdiff_state.base_file);
> -	ret = copyfile(img->fdin,
> -			&rdiff_state,
> -			img->size,
> -			(unsigned long *)&img->offset,
> -			img->seek,
> -			0, /* no skip */
> -			img->compressed,
> -			&img->checksum,
> -			img->sha256,
> -			img->is_encrypted,
> -			img->ivt_ascii,
> -			apply_rdiff_chunk_cb);
> +	ret = copyimage(&rdiff_state, img, apply_rdiff_chunk_cb);
>   	if (ret != 0) {
>   		ERROR("Error %d running rdiff job, aborting.", ret);
>   		goto cleanup;


Reviewed-by: Stefano Babic <stefano.babic@swupdate.org>
diff mbox series

Patch

diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c
index 03b74aed1cf2..10af0760c5f9 100644
--- a/corelib/lua_interface.c
+++ b/corelib/lua_interface.c
@@ -388,7 +388,7 @@  static int l_copy2file(lua_State *L)
 	}
 
 	struct img_type img = {};
-	uint32_t checksum = 0;
+	uint32_t image_checksum = img.checksum;
 
 	table2image(L, &img);
 	if (check_same_file(img.fdin, fdout)) {
@@ -397,18 +397,7 @@  static int l_copy2file(lua_State *L)
 		lua_pushstring(L, "Output file path is same as input file temporary path");
 		goto copyfile_exit;
 	}
-	int ret = copyfile(img.fdin,
-				 &fdout,
-				 img.size,
-				 (unsigned long *)&img.offset,
-				 img.seek,
-				 0, /* no skip */
-				 img.compressed,
-				 &checksum,
-				 img.sha256,
-				 img.is_encrypted,
-				 img.ivt_ascii,
-				 NULL);
+	int ret = copyimage(&fdout, &img, NULL /* default write callback */);
 	update_table(L, &img);
 	lua_pop(L, 1);
 
@@ -417,10 +406,10 @@  static int l_copy2file(lua_State *L)
 		lua_pushstring(L, strerror(errno));
 		goto copyfile_exit;
 	}
-	if ((img.checksum != 0) && (checksum != img.checksum)) {
+	if ((image_checksum != 0) && (image_checksum != img.checksum)) {
 		lua_pushinteger(L, -1);
 		lua_pushfstring(L, "Checksums WRONG! Computed 0x%d, should be 0x%d\n",
-						checksum, img.checksum);
+				img.checksum, image_checksum);
 		goto copyfile_exit;
 	}
 
@@ -463,24 +452,13 @@  static int l_istream_read(lua_State* L)
 	luaL_checktype(L, 2, LUA_TFUNCTION);
 
 	struct img_type img = {};
-	uint32_t checksum = 0;
+	uint32_t image_checksum = img.checksum;
 
 	lua_pushvalue(L, 1);
 	table2image(L, &img);
 	lua_pop(L, 1);
 
-	int ret = copyfile(img.fdin,
-				 L,
-				 img.size,
-				 (unsigned long *)&img.offset,
-				 img.seek,
-				 0, /* no skip */
-				 img.compressed,
-				 &checksum,
-				 img.sha256,
-				 img.is_encrypted,
-				 img.ivt_ascii,
-				 istream_read_callback);
+	int ret = copyimage(L, &img, istream_read_callback);
 
 	lua_pop(L, 1);
 	update_table(L, &img);
@@ -491,10 +469,10 @@  static int l_istream_read(lua_State* L)
 		lua_pushstring(L, strerror(errno));
 		return 2;
 	}
-	if ((img.checksum != 0) && (checksum != img.checksum)) {
+	if ((image_checksum != 0) && (image_checksum != img.checksum)) {
 		lua_pushinteger(L, -1);
 		lua_pushfstring(L, "Checksums WRONG! Computed 0x%d, should be 0x%d\n",
-						checksum, img.checksum);
+				img.checksum, image_checksum);
 		return 2;
 	}
 	lua_pushinteger(L, 0);
diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c
index 8e7faa0ead02..5ec243ee01d9 100644
--- a/handlers/delta_handler.c
+++ b/handlers/delta_handler.c
@@ -999,18 +999,7 @@  static int install_delta(struct img_type *img,
 		goto cleanup;
 	}
 
-	ret = copyfile(img->fdin,
-		&mem_fd,
-		img->size,
-		(unsigned long *)&img->offset,
-		img->seek,
-		0,
-		img->compressed,
-		&img->checksum,
-		img->sha256,
-		img->is_encrypted,
-		img->ivt_ascii,
-		NULL);
+	ret = copyimage(&mem_fd, img, NULL /* default write callback */);
 
 	if (ret != 0) {
 		ERROR("Error %d copying zchunk header, aborting.", ret);
diff --git a/handlers/rdiff_handler.c b/handlers/rdiff_handler.c
index e01a127bce50..3b01519ce80a 100644
--- a/handlers/rdiff_handler.c
+++ b/handlers/rdiff_handler.c
@@ -337,18 +337,7 @@  static int apply_rdiff_patch(struct img_type *img,
 	rs_trace_to(rdiff_log);
 
 	rdiff_state.job = rs_patch_begin(base_file_read_cb, rdiff_state.base_file);
-	ret = copyfile(img->fdin,
-			&rdiff_state,
-			img->size,
-			(unsigned long *)&img->offset,
-			img->seek,
-			0, /* no skip */
-			img->compressed,
-			&img->checksum,
-			img->sha256,
-			img->is_encrypted,
-			img->ivt_ascii,
-			apply_rdiff_chunk_cb);
+	ret = copyimage(&rdiff_state, img, apply_rdiff_chunk_cb);
 	if (ret != 0) {
 		ERROR("Error %d running rdiff job, aborting.", ret);
 		goto cleanup;