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 --- V2: Pass correct buffer to the flash_io() V3: Fix one more flash_io() call argument --- 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..a738b910c2 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.image, CUR_ENVSIZE)) { 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.image, 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.image, CUR_ENVSIZE)) { ret = -EIO; goto open_cleanup; }