public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: "Rafał Miłecki" <zajec5@gmail.com>
To: u-boot@lists.denx.de
Cc: "Rafał Miłecki" <rafal@milecki.pl>
Subject: [PATCH 1/2] fw_env: make flash_io() take buffer as an argument
Date: Mon, 10 Jan 2022 21:19:23 +0100	[thread overview]
Message-ID: <20220110201924.9411-2-zajec5@gmail.com> (raw)
In-Reply-To: <20220110201924.9411-1-zajec5@gmail.com>

From: Rafał Miłecki <rafal@milecki.pl>

It's usually easier to understand code & follow it if all arguments are
passed explicitly. Many coding styles also discourage using global
variables.

Behaviour of flash_io() was a bit unintuitive as it was writing to a
buffer referenced in a global struct. That required developers to
remember how it works and sometimes required hacking "environment"
global struct variable to read data into a proper buffer.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 tools/env/fw_env.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 3da75be783..f90293d7ce 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -346,7 +346,7 @@ static int ubi_write(int fd, const void *buf, size_t count)
 	return 0;
 }
 
-static int flash_io(int mode);
+static int flash_io(int mode, void *buf, size_t count);
 static int parse_config(struct env_opts *opts);
 
 #if defined(CONFIG_FILE)
@@ -516,7 +516,7 @@ int fw_env_flush(struct env_opts *opts)
 	*environment.crc = crc32(0, (uint8_t *) environment.data, ENV_SIZE);
 
 	/* write environment back to flash */
-	if (flash_io(O_RDWR)) {
+	if (flash_io(O_RDWR, environment.data, ENV_SIZE)) {
 		fprintf(stderr, "Error: can't write fw_env to flash\n");
 		return -1;
 	}
@@ -1185,7 +1185,8 @@ static int flash_flag_obsolete(int dev, int fd, off_t offset)
 	return rc;
 }
 
-static int flash_write(int fd_current, int fd_target, int dev_target)
+static int flash_write(int fd_current, int fd_target, int dev_target, void *buf,
+		       size_t count)
 {
 	int rc;
 
@@ -1212,11 +1213,10 @@ static int flash_write(int fd_current, int fd_target, int dev_target)
 	if (IS_UBI(dev_target)) {
 		if (ubi_update_start(fd_target, CUR_ENVSIZE) < 0)
 			return -1;
-		return ubi_write(fd_target, environment.image, CUR_ENVSIZE);
+		return ubi_write(fd_target, buf, count);
 	}
 
-	rc = flash_write_buf(dev_target, fd_target, environment.image,
-			     CUR_ENVSIZE);
+	rc = flash_write_buf(dev_target, fd_target, buf, count);
 	if (rc < 0)
 		return rc;
 
@@ -1235,17 +1235,17 @@ static int flash_write(int fd_current, int fd_target, int dev_target)
 	return 0;
 }
 
-static int flash_read(int fd)
+static int flash_read(int fd, void *buf, size_t count)
 {
 	int rc;
 
 	if (IS_UBI(dev_current)) {
 		DEVTYPE(dev_current) = MTD_ABSENT;
 
-		return ubi_read(fd, environment.image, CUR_ENVSIZE);
+		return ubi_read(fd, buf, count);
 	}
 
-	rc = flash_read_buf(dev_current, fd, environment.image, CUR_ENVSIZE,
+	rc = flash_read_buf(dev_current, fd, buf, count,
 			    DEVOFFSET(dev_current));
 	if (rc != CUR_ENVSIZE)
 		return -1;
@@ -1291,7 +1291,7 @@ err:
 	return rc;
 }
 
-static int flash_io_write(int fd_current)
+static int flash_io_write(int fd_current, void *buf, size_t count)
 {
 	int fd_target = -1, rc, dev_target;
 	const char *dname, *target_temp = NULL;
@@ -1322,7 +1322,7 @@ static int flash_io_write(int fd_current)
 			fd_target = fd_current;
 	}
 
-	rc = flash_write(fd_current, fd_target, dev_target);
+	rc = flash_write(fd_current, fd_target, dev_target, buf, count);
 
 	if (fsync(fd_current) && !(errno == EINVAL || errno == EROFS)) {
 		fprintf(stderr,
@@ -1377,7 +1377,7 @@ static int flash_io_write(int fd_current)
 	return rc;
 }
 
-static int flash_io(int mode)
+static int flash_io(int mode, void *buf, size_t count)
 {
 	int fd_current, rc;
 
@@ -1391,9 +1391,9 @@ static int flash_io(int mode)
 	}
 
 	if (mode == O_RDWR) {
-		rc = flash_io_write(fd_current);
+		rc = flash_io_write(fd_current, buf, count);
 	} else {
-		rc = flash_read(fd_current);
+		rc = flash_read(fd_current, buf, count);
 	}
 
 	if (close(fd_current)) {
@@ -1455,7 +1455,7 @@ int fw_env_open(struct env_opts *opts)
 	}
 
 	dev_current = 0;
-	if (flash_io(O_RDONLY)) {
+	if (flash_io(O_RDONLY, environment.data, CUR_ENVSIZE)) {
 		ret = -EIO;
 		goto open_cleanup;
 	}
@@ -1490,7 +1490,7 @@ int fw_env_open(struct env_opts *opts)
 		 * other pointers in environment still point inside addr0
 		 */
 		environment.image = addr1;
-		if (flash_io(O_RDONLY)) {
+		if (flash_io(O_RDONLY, environment.data, CUR_ENVSIZE)) {
 			ret = -EIO;
 			goto open_cleanup;
 		}
-- 
2.31.1


  reply	other threads:[~2022-01-11  2:00 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-10 20:19 [PATCH 0/2] fw_env: two minor code cleanups Rafał Miłecki
2022-01-10 20:19 ` Rafał Miłecki [this message]
2022-01-11 14:10   ` [PATCH 1/2] fw_env: make flash_io() take buffer as an argument Rafał Miłecki
2022-01-10 20:19 ` [PATCH 2/2] fw_env: simplify logic & code paths in the fw_env_open() Rafał Miłecki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220110201924.9411-2-zajec5@gmail.com \
    --to=zajec5@gmail.com \
    --cc=rafal@milecki.pl \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox