[U-Boot] [PATCH v2 4/4] env_sf: use DIV_ROUND_UP to calculate number of sectors to erase

[Andreas Fenkart]

simpler to read

Signed-off-by: Andreas Fenkart <afenk...@gmail.com>
---
 common/env_sf.c | 18 +-
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/common/env_sf.c b/common/env_sf.c
index 6a1583ebec..9944602367 100644
--- a/common/env_sf.c
+++ b/common/env_sf.c
@@ -80,7 +80,7 @@ int saveenv(void)
 {
env_t   env_new;
char*saved_buffer = NULL, flag = OBSOLETE_FLAG;
-   u32 saved_size, saved_offset, sector = 1;
+   u32 saved_size, saved_offset, sector;
int ret;
 
ret = setup_flash_device();
@@ -115,11 +115,7 @@ int saveenv(void)
goto done;
}
 
-   if (CONFIG_ENV_SIZE > CONFIG_ENV_SECT_SIZE) {
-   sector = CONFIG_ENV_SIZE / CONFIG_ENV_SECT_SIZE;
-   if (CONFIG_ENV_SIZE % CONFIG_ENV_SECT_SIZE)
-   sector++;
-   }
+   sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
 
puts("Erasing SPI flash...");
ret = spi_flash_erase(env_flash, env_new_offset,
@@ -245,7 +241,7 @@ out:
 #else
 int saveenv(void)
 {
-   u32 saved_size, saved_offset, sector = 1;
+   u32 saved_size, saved_offset, sector;
char*saved_buffer = NULL;
int ret = 1;
env_t   env_new;
@@ -268,16 +264,12 @@ int saveenv(void)
goto done;
}
 
-   if (CONFIG_ENV_SIZE > CONFIG_ENV_SECT_SIZE) {
-   sector = CONFIG_ENV_SIZE / CONFIG_ENV_SECT_SIZE;
-   if (CONFIG_ENV_SIZE % CONFIG_ENV_SECT_SIZE)
-   sector++;
-   }
-
ret = env_export(_new);
if (ret)
goto done;
 
+   sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
+
puts("Erasing SPI flash...");
ret = spi_flash_erase(env_flash, CONFIG_ENV_OFFSET,
sector * CONFIG_ENV_SECT_SIZE);
-- 
2.11.0

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2 0/4] env_sf: minor refactorings

[Andreas Fenkart]

rebased patches on master
fixed compiled error in 1st patch (missing variable declaration)
compile tested all patches with travis

Andreas Fenkart (4):
  env_sf: factor out prepare_flash_device
  enf_sf: reuse setup_flash_device instead of open coding it
  env_sf: re-order error handling in single-buffer env_relocate_spec
  env_sf: use DIV_ROUND_UP to calculate number of sectors to erase

 common/env_sf.c | 92 ++---
 1 file changed, 36 insertions(+), 56 deletions(-)

-- 
2.11.0

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2 3/4] env_sf: re-order error handling in single-buffer env_relocate_spec

[Andreas Fenkart]

this makes it easier comparable to the double-buffered version

Signed-off-by: Andreas Fenkart <afenk...@gmail.com>
Reviewed-by: Simon Glass <s...@chromium.org>
---
 common/env_sf.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/common/env_sf.c b/common/env_sf.c
index a52fb734c8..6a1583ebec 100644
--- a/common/env_sf.c
+++ b/common/env_sf.c
@@ -313,29 +313,31 @@ void env_relocate_spec(void)
char *buf = NULL;
 
buf = (char *)memalign(ARCH_DMA_MINALIGN, CONFIG_ENV_SIZE);
-
-   ret = setup_flash_device();
-   if (ret) {
-   if (buf)
-   free(buf);
+   if (!buf) {
+   set_default_env("!malloc() failed");
return;
}
 
+   ret = setup_flash_device();
+   if (ret)
+   goto out;
+
ret = spi_flash_read(env_flash,
CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE, buf);
if (ret) {
set_default_env("!spi_flash_read() failed");
-   goto out;
+   goto err_read;
}
 
ret = env_import(buf, 1);
if (ret)
gd->env_valid = 1;
-out:
+
+err_read:
spi_flash_free(env_flash);
-   if (buf)
-   free(buf);
env_flash = NULL;
+out:
+   free(buf);
 }
 #endif
 
-- 
2.11.0

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2 2/4] enf_sf: reuse setup_flash_device instead of open coding it

[Andreas Fenkart]

setup_flash_device selects one of two code paths depending on the driver
model being used (=CONFIG_DM_SPI_FLASH). env_relocate_spec only used
the non driver-model code path. I'm unsure why, either none of the
platforms that need relocation use the driver model, or - worse - the
driver model is not yet usable when relocating.

Signed-off-by: Andreas Fenkart <afenk...@gmail.com>
---
 common/env_sf.c | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/common/env_sf.c b/common/env_sf.c
index 8af590a3d9..a52fb734c8 100644
--- a/common/env_sf.c
+++ b/common/env_sf.c
@@ -176,12 +176,9 @@ void env_relocate_spec(void)
goto out;
}
 
-   env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
-   CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);
-   if (!env_flash) {
-   set_default_env("!spi_flash_probe() failed");
+   ret = setup_flash_device();
+   if (ret)
goto out;
-   }
 
ret = spi_flash_read(env_flash, CONFIG_ENV_OFFSET,
CONFIG_ENV_SIZE, tmp_env1);
@@ -316,10 +313,9 @@ void env_relocate_spec(void)
char *buf = NULL;
 
buf = (char *)memalign(ARCH_DMA_MINALIGN, CONFIG_ENV_SIZE);
-   env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
-   CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);
-   if (!env_flash) {
-   set_default_env("!spi_flash_probe() failed");
+
+   ret = setup_flash_device();
+   if (ret) {
if (buf)
free(buf);
return;
-- 
2.11.0

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2 1/4] env_sf: factor out prepare_flash_device

[Andreas Fenkart]

copy code found in single/double buffered code path

Signed-off-by: Andreas Fenkart <afenk...@gmail.com>
Reviewed-by: Simon Glass <s...@chromium.org>
---
 common/env_sf.c | 48 +++-
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/common/env_sf.c b/common/env_sf.c
index 27b4d1226a..8af590a3d9 100644
--- a/common/env_sf.c
+++ b/common/env_sf.c
@@ -45,15 +45,11 @@ char *env_name_spec = "SPI Flash";
 
 static struct spi_flash *env_flash;
 
-#if defined(CONFIG_ENV_OFFSET_REDUND)
-int saveenv(void)
+static int setup_flash_device(void)
 {
-   env_t   env_new;
-   char*saved_buffer = NULL, flag = OBSOLETE_FLAG;
-   u32 saved_size, saved_offset, sector = 1;
-   int ret;
 #ifdef CONFIG_DM_SPI_FLASH
struct udevice *new;
+   int ret;
 
/* speed and mode will be read from DT */
ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
@@ -76,6 +72,20 @@ int saveenv(void)
}
}
 #endif
+   return 0;
+}
+
+#if defined(CONFIG_ENV_OFFSET_REDUND)
+int saveenv(void)
+{
+   env_t   env_new;
+   char*saved_buffer = NULL, flag = OBSOLETE_FLAG;
+   u32 saved_size, saved_offset, sector = 1;
+   int ret;
+
+   ret = setup_flash_device();
+   if (ret)
+   return ret;
 
ret = env_export(_new);
if (ret)
@@ -242,30 +252,10 @@ int saveenv(void)
char*saved_buffer = NULL;
int ret = 1;
env_t   env_new;
-#ifdef CONFIG_DM_SPI_FLASH
-   struct udevice *new;
 
-   /* speed and mode will be read from DT */
-   ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
-0, 0, );
-   if (ret) {
-   set_default_env("!spi_flash_probe_bus_cs() failed");
-   return 1;
-   }
-
-   env_flash = dev_get_uclass_priv(new);
-#else
-
-   if (!env_flash) {
-   env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS,
-   CONFIG_ENV_SPI_CS,
-   CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);
-   if (!env_flash) {
-   set_default_env("!spi_flash_probe() failed");
-   return 1;
-   }
-   }
-#endif
+   ret = setup_flash_device();
+   if (ret)
+   return ret;
 
/* Is the sector larger than the env (i.e. embedded) */
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
-- 
2.11.0

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2] tools/env: fix environment alignment tests for block devices

[Andreas Fenkart]

Hi Max,

LGTM, see one nit below, can fixed later

On 11/19/2016 01:58 PM, Max Krummenacher wrote:

commit 183923d3e412500bdc597d1745e2fb6f7f679ec7 enforces that the
environment must start at an erase block boundary.

For block devices the sample fw_env.config does not mandate a erase block size
for block devices. A missing setting defaults to the full env size.

Depending on the environment location the alignment check now errors out for
perfectly legal settings.

Fix this by defaulting to the standard blocksize of 0x200 for environments
stored in a block device.
That keeps the fw_env.config files for block devices working even with that
new check.

Signed-off-by: Max Krummenacher 

---

Changes in v2:
- move default value handling from parse_config(), get_config() to
   check_device_config(), so that !defined(CONFIG_FILE) is covered also.
- use DIV_ROUND_UP instead of doing this manually
- move the check after the setting of default values in check_device_config().
- use 0 as the marker for default values to be used.

  tools/env/fw_env.c | 60 ++
  1 file changed, 33 insertions(+), 27 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 3dc0d53..862a0b1 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -1291,18 +1291,6 @@ static int check_device_config(int dev)
struct stat st;
int fd, rc = 0;
  
-	if (DEVOFFSET(dev) % DEVESIZE(dev) != 0) {

-   fprintf(stderr, "Environment does not start on (erase) block 
boundary\n");
-   errno = EINVAL;
-   return -1;
-   }
-
-   if (ENVSIZE(dev) > ENVSECTORS(dev) * DEVESIZE(dev)) {
-   fprintf(stderr, "Environment does not fit into available 
sectors\n");
-   errno = EINVAL;
-   return -1;
-   }
-
fd = open(DEVNAME(dev), O_RDONLY);
if (fd < 0) {
fprintf(stderr,
@@ -1335,9 +1323,15 @@ static int check_device_config(int dev)
goto err;
}
DEVTYPE(dev) = mtdinfo.type;
+   if (DEVESIZE(dev) == 0)
+   /* Assume the erase size is the same as the env-size */
+   DEVESIZE(dev) = ENVSIZE(dev);
Since we already checked for character devices, we could use the 
mtdinfo.erasesize. I guess we can relay on mtdinfo on new kernels, and 
if not there is still the fallback to specify it in fw_env.config.



} else {
uint64_t size;
DEVTYPE(dev) = MTD_ABSENT;
+   if (DEVESIZE(dev) == 0)
+   /* Assume the erase size to be 512 bytes */
+   DEVESIZE(dev) = 0x200;
It doesn't even matter. In case of MTD_ABSENT, erasize is never used 
itself, only the tuple (DEVESIZE * ENVSECTORS) matters.


  
  		/*

 * Check for negative offsets, treat it as backwards offset
@@ -1359,6 +1353,22 @@ static int check_device_config(int dev)
}
}
  
+	if (ENVSECTORS(dev) == 0)

+   /* Assume enough sectors to cover the environment */
+   ENVSECTORS(dev) = DIV_ROUND_UP(ENVSIZE(dev), DEVESIZE(dev));
+
+   if (DEVOFFSET(dev) % DEVESIZE(dev) != 0) {
+   fprintf(stderr, "Environment does not start on (erase) block 
boundary\n");
+   errno = EINVAL;
+   return -1;
+   }
+
+   if (ENVSIZE(dev) > ENVSECTORS(dev) * DEVESIZE(dev)) {
+   fprintf(stderr, "Environment does not fit into available 
sectors\n");
+   errno = EINVAL;
+   return -1;
+   }
+
  err:
close(fd);
return rc;
@@ -1382,10 +1392,10 @@ static int parse_config(struct env_opts *opts)
DEVNAME (0) = DEVICE1_NAME;
DEVOFFSET (0) = DEVICE1_OFFSET;
ENVSIZE (0) = ENV1_SIZE;
-   /* Default values are: erase-size=env-size */
-   DEVESIZE (0) = ENVSIZE (0);
-   /* #sectors=env-size/erase-size (rounded up) */
-   ENVSECTORS (0) = (ENVSIZE(0) + DEVESIZE(0) - 1) / DEVESIZE(0);
+
+   /* Set defaults for DEVESIZE, ENVSECTORS later once we
+* know DEVTYPE
+*/
  #ifdef DEVICE1_ESIZE
DEVESIZE (0) = DEVICE1_ESIZE;
  #endif
@@ -1397,10 +1407,10 @@ static int parse_config(struct env_opts *opts)
DEVNAME (1) = DEVICE2_NAME;
DEVOFFSET (1) = DEVICE2_OFFSET;
ENVSIZE (1) = ENV2_SIZE;
-   /* Default values are: erase-size=env-size */
-   DEVESIZE (1) = ENVSIZE (1);
-   /* #sectors=env-size/erase-size (rounded up) */
-   ENVSECTORS (1) = (ENVSIZE(1) + DEVESIZE(1) - 1) / DEVESIZE(1);
+
+   /* Set defaults for DEVESIZE, ENVSECTORS later once we
+* know DEVTYPE
+*/
  #ifdef DEVICE2_ESIZE
DEVESIZE (1) = DEVICE2_ESIZE;
  #endif
@@ -1466,13 +1476,9 @@ static int get_config (char *fname)
  
  		DEVNAME(i) = devname;
  
-		if (rc < 4)

-   /* 

[U-Boot] [PATCH 4/4] env_sf: use DIV_ROUND_UP to calculate number of sectors to erase

[Andreas Fenkart]

simpler, needs less thinking when reading the code

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 common/env_sf.c | 18 +-
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/common/env_sf.c b/common/env_sf.c
index 8a3de63..0434bb8 100644
--- a/common/env_sf.c
+++ b/common/env_sf.c
@@ -79,7 +79,7 @@ int saveenv(void)
 {
env_t   env_new;
char*saved_buffer = NULL, flag = OBSOLETE_FLAG;
-   u32 saved_size, saved_offset, sector = 1;
+   u32 saved_size, saved_offset, sector;
int ret;
 
ret = setup_flash_device();
@@ -114,11 +114,7 @@ int saveenv(void)
goto done;
}
 
-   if (CONFIG_ENV_SIZE > CONFIG_ENV_SECT_SIZE) {
-   sector = CONFIG_ENV_SIZE / CONFIG_ENV_SECT_SIZE;
-   if (CONFIG_ENV_SIZE % CONFIG_ENV_SECT_SIZE)
-   sector++;
-   }
+   sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
 
puts("Erasing SPI flash...");
ret = spi_flash_erase(env_flash, env_new_offset,
@@ -244,7 +240,7 @@ out:
 #else
 int saveenv(void)
 {
-   u32 saved_size, saved_offset, sector = 1;
+   u32 saved_size, saved_offset, sector;
char*saved_buffer = NULL;
int ret = 1;
env_t   env_new;
@@ -267,16 +263,12 @@ int saveenv(void)
goto done;
}
 
-   if (CONFIG_ENV_SIZE > CONFIG_ENV_SECT_SIZE) {
-   sector = CONFIG_ENV_SIZE / CONFIG_ENV_SECT_SIZE;
-   if (CONFIG_ENV_SIZE % CONFIG_ENV_SECT_SIZE)
-   sector++;
-   }
-
ret = env_export(_new);
if (ret)
goto done;
 
+   sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
+
puts("Erasing SPI flash...");
ret = spi_flash_erase(env_flash, CONFIG_ENV_OFFSET,
sector * CONFIG_ENV_SECT_SIZE);
-- 
2.10.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/4] enf_sf: reuse setup_flash_device instead of open coding it

[Andreas Fenkart]

setup_flash_device selects one of two code paths depending on the driver
model being used (=CONFIG_DM_SPI_FLASH). env_relocate_spec only used
the non driver-model code path. I'm unsure why, either none of the
platforms that need relocation use the driver model, or - worse - the
driver model is not yet usable when relocating.

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 common/env_sf.c | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/common/env_sf.c b/common/env_sf.c
index 5126762..ba9ac8a 100644
--- a/common/env_sf.c
+++ b/common/env_sf.c
@@ -175,12 +175,9 @@ void env_relocate_spec(void)
goto out;
}
 
-   env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
-   CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);
-   if (!env_flash) {
-   set_default_env("!spi_flash_probe() failed");
+   ret = setup_flash_device();
+   if (ret)
goto out;
-   }
 
ret = spi_flash_read(env_flash, CONFIG_ENV_OFFSET,
CONFIG_ENV_SIZE, tmp_env1);
@@ -315,10 +312,9 @@ void env_relocate_spec(void)
char *buf = NULL;
 
buf = (char *)memalign(ARCH_DMA_MINALIGN, CONFIG_ENV_SIZE);
-   env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
-   CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);
-   if (!env_flash) {
-   set_default_env("!spi_flash_probe() failed");
+
+   ret = setup_flash_device();
+   if (ret) {
if (buf)
free(buf);
return;
-- 
2.10.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/4] env_sf: factor out prepare_flash_device

[Andreas Fenkart]

copy code found in single/double buffered code path

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 common/env_sf.c | 47 ++-
 1 file changed, 18 insertions(+), 29 deletions(-)

diff --git a/common/env_sf.c b/common/env_sf.c
index c53200f..5126762 100644
--- a/common/env_sf.c
+++ b/common/env_sf.c
@@ -45,13 +45,8 @@ char *env_name_spec = "SPI Flash";
 
 static struct spi_flash *env_flash;
 
-#if defined(CONFIG_ENV_OFFSET_REDUND)
-int saveenv(void)
+static int setup_flash_device(void)
 {
-   env_t   env_new;
-   char*saved_buffer = NULL, flag = OBSOLETE_FLAG;
-   u32 saved_size, saved_offset, sector = 1;
-   int ret;
 #ifdef CONFIG_DM_SPI_FLASH
struct udevice *new;
 
@@ -76,6 +71,20 @@ int saveenv(void)
}
}
 #endif
+   return 0;
+}
+
+#if defined(CONFIG_ENV_OFFSET_REDUND)
+int saveenv(void)
+{
+   env_t   env_new;
+   char*saved_buffer = NULL, flag = OBSOLETE_FLAG;
+   u32 saved_size, saved_offset, sector = 1;
+   int ret;
+
+   ret = setup_flash_device();
+   if (ret)
+   return ret;
 
ret = env_export(_new);
if (ret)
@@ -242,30 +251,10 @@ int saveenv(void)
char*saved_buffer = NULL;
int ret = 1;
env_t   env_new;
-#ifdef CONFIG_DM_SPI_FLASH
-   struct udevice *new;
-
-   /* speed and mode will be read from DT */
-   ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
-0, 0, );
-   if (ret) {
-   set_default_env("!spi_flash_probe_bus_cs() failed");
-   return 1;
-   }
 
-   env_flash = dev_get_uclass_priv(new);
-#else
-
-   if (!env_flash) {
-   env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS,
-   CONFIG_ENV_SPI_CS,
-   CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);
-   if (!env_flash) {
-   set_default_env("!spi_flash_probe() failed");
-   return 1;
-   }
-   }
-#endif
+   ret = setup_flash_device();
+   if (ret)
+   return ret;
 
/* Is the sector larger than the env (i.e. embedded) */
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
-- 
2.10.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 3/4] env_sf: re-order error handling in single-buffer env_relocate_spec

[Andreas Fenkart]

this makes it easier comparable to the double-buffered version

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 common/env_sf.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/common/env_sf.c b/common/env_sf.c
index ba9ac8a..8a3de63 100644
--- a/common/env_sf.c
+++ b/common/env_sf.c
@@ -312,29 +312,31 @@ void env_relocate_spec(void)
char *buf = NULL;
 
buf = (char *)memalign(ARCH_DMA_MINALIGN, CONFIG_ENV_SIZE);
-
-   ret = setup_flash_device();
-   if (ret) {
-   if (buf)
-   free(buf);
+   if (!buf) {
+   set_default_env("!malloc() failed");
return;
}
 
+   ret = setup_flash_device();
+   if (ret)
+   goto out;
+
ret = spi_flash_read(env_flash,
CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE, buf);
if (ret) {
set_default_env("!spi_flash_read() failed");
-   goto out;
+   goto err_read;
}
 
ret = env_import(buf, 1);
if (ret)
gd->env_valid = 1;
-out:
+
+err_read:
spi_flash_free(env_flash);
-   if (buf)
-   free(buf);
env_flash = NULL;
+out:
+   free(buf);
 }
 #endif
 
-- 
2.10.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 0/4] env_sf: minor cleanup

[Andreas Fenkart]

Andreas Fenkart (4):
  env_sf: factor out prepare_flash_device
  enf_sf: reuse setup_flash_device instead of open coding it
  env_sf: re-order error handling in single-buffer env_relocate_spec
  env_sf: use DIV_ROUND_UP to calculate number of sectors to erase

 common/env_sf.c | 91 ++---
 1 file changed, 35 insertions(+), 56 deletions(-)

-- 
2.10.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] tools/env: fix environment alignment tests for block devices

[Andreas Fenkart]

2016-11-18 11:38 GMT+01:00 Max Krummenacher :
> commit 183923d3e412500bdc597d1745e2fb6f7f679ec7 enforces that the
> environment must start at an erase block boundary.
>
> For block devices the sample fw_env.config does not mandate a erase block size
> for block devices. A missing setting defaults to the full env size.
>
> Depending on the environment location the alignment check now errors out for
> perfectly legal settings.
>
> Fix this by defaulting to the standard blocksize of 0x200 for environments
> stored in a block device.
> That keeps the fw_env.config files for block devices working even with that
> new check.
>
> Signed-off-by: Max Krummenacher 
> ---
>
>  tools/env/fw_env.c | 22 ++
>  1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> index 3dc0d53..f2126f8 100644
> --- a/tools/env/fw_env.c
> +++ b/tools/env/fw_env.c
> @@ -1335,10 +1335,25 @@ static int check_device_config(int dev)
> goto err;
> }
> DEVTYPE(dev) = mtdinfo.type;

There is 'erasesize' in that struct, not sure how reliable that is and
if we shall rely on it. Any experiences on this?
In general we should take that and in case it is wrong the user has to
overwrite it in fw_env.config

# mtdinfo /dev/mtd5
mtd5
Name:   spi32766.0
Type:   nor
Eraseblock size:65536 bytes, 64.0 KiB
Amount of eraseblocks:  8 (524288 bytes, 512.0 KiB)
Minimum input/output unit size: 1 byte
Sub-page size:  1 byte
Character device major/minor:   90:10
Bad blocks are allowed: false
Device is writable: true

> +   if (DEVESIZE(dev) == -1) {
> +   /* Assume the erase size is the same as the env-size 
> */
> +   DEVESIZE(dev) = ENVSIZE(dev);
> +   /* Assume enough env sectors to cover the environment 
> */
> +   ENVSECTORS(dev) = (ENVSIZE(dev) + DEVESIZE(dev) - 1) /
> +  DEVESIZE(dev);

DIV_ROUND_UP

> +   }
> } else {
> uint64_t size;
> DEVTYPE(dev) = MTD_ABSENT;
>
> +   if (DEVESIZE(dev) == -1) {
> +   /* Assume the erase size to be 512 bytes */
> +   DEVESIZE(dev) = 0x200;
> +   /* Assume enough env sectors to cover the environment 
> */
> +   ENVSECTORS(dev) = (ENVSIZE(dev) + DEVESIZE(dev) - 1) /
> +  DEVESIZE(dev);

DIV_ROUND_UP

> +   }
> +
> /*
>  * Check for negative offsets, treat it as backwards offset
>  * from the end of the block device
> @@ -1467,10 +1482,9 @@ static int get_config (char *fname)
> DEVNAME(i) = devname;
>
> if (rc < 4)
> -   /* Assume the erase size is the same as the env-size 
> */
> -   DEVESIZE(i) = ENVSIZE(i);
> -
> -   if (rc < 5)
> +   /* Fixup later depending on DEVTYPE */
> +   DEVESIZE(i) = -1;

I think we can rely on '0', since that is invalid already.


> +   else if (rc < 5)
> /* Assume enough env sectors to cover the environment 
> */
> ENVSECTORS (i) = (ENVSIZE(i) + DEVESIZE(i) - 1) / 
> DEVESIZE(i);

I guess the !defined(CONFIG_FILE) has the same problem. Let's just
ignore the cases rc < 4,5 here and handle them all in check_config.

>
> --
> 2.5.5
>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] Suspected Spam: Do not open attachements![PATCH 4/6] tools/env: flash_write_buf: enforce offset to be start of environment

[Andreas Fenkart]

This allows to take advantage of the environment being block aligned.
This is not a new constraint. Writes always start at the begin of the
environment, since the header with CRC/length as there.
Every environment modification requires updating the header

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 0f0eaa4..3dc0d53 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -765,12 +765,12 @@ static int flash_read_buf (int dev, int fd, void *buf, 
size_t count,
 }
 
 /*
- * Write count bytes at offset, but stay within ENVSECTORS (dev) sectors of
+ * Write count bytes from begin of environment, but stay within
+ * ENVSECTORS(dev) sectors of
  * DEVOFFSET (dev). Similar to the read case above, on NOR and dataflash we
  * erase and write the whole data at once.
  */
-static int flash_write_buf (int dev, int fd, void *buf, size_t count,
-   off_t offset)
+static int flash_write_buf(int dev, int fd, void *buf, size_t count)
 {
void *data;
struct erase_info_user erase;
@@ -796,20 +796,21 @@ static int flash_write_buf (int dev, int fd, void *buf, 
size_t count,
if (DEVTYPE(dev) == MTD_ABSENT) {
blocklen = count;
erase_len = blocklen;
-   blockstart = offset;
+   blockstart = DEVOFFSET(dev);
block_seek = 0;
write_total = blocklen;
} else {
blocklen = DEVESIZE(dev);
 
-   erase_offset = (offset / blocklen) * blocklen;
+   erase_offset = DEVOFFSET(dev);
 
/* Maximum area we may use */
erase_len = environment_end(dev) - erase_offset;
 
blockstart = erase_offset;
+
/* Offset inside a block */
-   block_seek = offset - erase_offset;
+   block_seek = DEVOFFSET(dev) - erase_offset;
 
/*
 * Data size we actually write: from the start of the block
@@ -1007,7 +1008,7 @@ static int flash_write (int fd_current, int fd_target, 
int dev_target)
 #endif
 
rc = flash_write_buf(dev_target, fd_target, environment.image,
-CUR_ENVSIZE, DEVOFFSET(dev_target));
+CUR_ENVSIZE);
if (rc < 0)
return rc;
 
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 3/6] tools/env: lookup dev_type directly from flash_read_buf/flash_write_buf

[Andreas Fenkart]

flash_write_buf already looks up size/offset/#sector from struct
envdev_s. It can look up mtd_type as well. Same applies to
flash_read_buf. Makes the interface simpler

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 28 +---
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 86849b7..0f0eaa4 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -689,7 +689,7 @@ static int flash_bad_block(int fd, uint8_t mtd_type, loff_t 
blockstart)
  * the DEVOFFSET (dev) block. On NOR the loop is only run once.
  */
 static int flash_read_buf (int dev, int fd, void *buf, size_t count,
-  off_t offset, uint8_t mtd_type)
+  off_t offset)
 {
size_t blocklen;/* erase / write length - one block on NAND,
   0 on NOR */
@@ -706,7 +706,7 @@ static int flash_read_buf (int dev, int fd, void *buf, 
size_t count,
/* Offset inside a block */
block_seek = offset - blockstart;
 
-   if (mtd_type == MTD_NANDFLASH) {
+   if (DEVTYPE(dev) == MTD_NANDFLASH) {
/*
 * NAND: calculate which blocks we are reading. We have
 * to read one block at a time to skip bad blocks.
@@ -722,7 +722,7 @@ static int flash_read_buf (int dev, int fd, void *buf, 
size_t count,
 
/* This only runs once on NOR flash */
while (processed < count) {
-   rc = flash_bad_block(fd, mtd_type, blockstart);
+   rc = flash_bad_block(fd, DEVTYPE(dev), blockstart);
if (rc < 0) /* block test failed */
return -1;
 
@@ -770,7 +770,7 @@ static int flash_read_buf (int dev, int fd, void *buf, 
size_t count,
  * erase and write the whole data at once.
  */
 static int flash_write_buf (int dev, int fd, void *buf, size_t count,
-   off_t offset, uint8_t mtd_type)
+   off_t offset)
 {
void *data;
struct erase_info_user erase;
@@ -793,7 +793,7 @@ static int flash_write_buf (int dev, int fd, void *buf, 
size_t count,
/*
 * For mtd devices only offset and size of the environment do matter
 */
-   if (mtd_type == MTD_ABSENT) {
+   if (DEVTYPE(dev) == MTD_ABSENT) {
blocklen = count;
erase_len = blocklen;
blockstart = offset;
@@ -834,8 +834,7 @@ static int flash_write_buf (int dev, int fd, void *buf, 
size_t count,
return -1;
}
 
-   rc = flash_read_buf (dev, fd, data, write_total, erase_offset,
-mtd_type);
+   rc = flash_read_buf(dev, fd, data, write_total, erase_offset);
if (write_total != rc)
return -1;
 
@@ -862,7 +861,7 @@ static int flash_write_buf (int dev, int fd, void *buf, 
size_t count,
data = buf;
}
 
-   if (mtd_type == MTD_NANDFLASH) {
+   if (DEVTYPE(dev) == MTD_NANDFLASH) {
/*
 * NAND: calculate which blocks we are writing. We have
 * to write one block at a time to skip bad blocks.
@@ -876,7 +875,7 @@ static int flash_write_buf (int dev, int fd, void *buf, 
size_t count,
 
/* This only runs once on NOR flash and SPI-dataflash */
while (processed < write_total) {
-   rc = flash_bad_block(fd, mtd_type, blockstart);
+   rc = flash_bad_block(fd, DEVTYPE(dev), blockstart);
if (rc < 0) /* block test failed */
return rc;
 
@@ -890,11 +889,11 @@ static int flash_write_buf (int dev, int fd, void *buf, 
size_t count,
continue;
}
 
-   if (mtd_type != MTD_ABSENT) {
+   if (DEVTYPE(dev) != MTD_ABSENT) {
erase.start = blockstart;
ioctl(fd, MEMUNLOCK, );
/* These do not need an explicit erase cycle */
-   if (mtd_type != MTD_DATAFLASH)
+   if (DEVTYPE(dev) != MTD_DATAFLASH)
if (ioctl(fd, MEMERASE, ) != 0) {
fprintf(stderr,
"MTD erase error on %s: %s\n",
@@ -921,7 +920,7 @@ static int flash_write_buf (int dev, int fd, void *buf, 
size_t count,
return -1;
}
 
-   if (mtd_type != MTD_ABSENT)
+   if (DEVTYPE(dev) != MTD_ABSENT)
ioctl(fd, MEMLOCK, );
 
processed  += erasesize;
@@ -1008,8 +1007,7 @@ static int flash_write (int fd_current, int fd_target, 
int dev_target)
 #endif
 
rc = flash_write_buf(dev_target, 

[U-Boot] [PATCH 2/6] tools/env: pass bad block offset by value

[Andreas Fenkart]

the offset is not modified by linux ioctl call
see mtd_ioctl{drivers/mtd/mtdchar.c}
Makes the interface less ambiguous, since the caller can
now exclude a modification of blockstart

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 90eb5fa..86849b7 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -661,10 +661,10 @@ off_t environment_end(int dev)
  * > 0 - block is bad
  * < 0 - failed to test
  */
-static int flash_bad_block (int fd, uint8_t mtd_type, loff_t *blockstart)
+static int flash_bad_block(int fd, uint8_t mtd_type, loff_t blockstart)
 {
if (mtd_type == MTD_NANDFLASH) {
-   int badblock = ioctl (fd, MEMGETBADBLOCK, blockstart);
+   int badblock = ioctl(fd, MEMGETBADBLOCK, );
 
if (badblock < 0) {
perror ("Cannot read bad block mark");
@@ -674,7 +674,7 @@ static int flash_bad_block (int fd, uint8_t mtd_type, 
loff_t *blockstart)
if (badblock) {
 #ifdef DEBUG
fprintf (stderr, "Bad block at 0x%llx, skipping\n",
-   (unsigned long long) *blockstart);
+   (unsigned long long)blockstart);
 #endif
return badblock;
}
@@ -722,7 +722,7 @@ static int flash_read_buf (int dev, int fd, void *buf, 
size_t count,
 
/* This only runs once on NOR flash */
while (processed < count) {
-   rc = flash_bad_block (fd, mtd_type, );
+   rc = flash_bad_block(fd, mtd_type, blockstart);
if (rc < 0) /* block test failed */
return -1;
 
@@ -876,7 +876,7 @@ static int flash_write_buf (int dev, int fd, void *buf, 
size_t count,
 
/* This only runs once on NOR flash and SPI-dataflash */
while (processed < write_total) {
-   rc = flash_bad_block (fd, mtd_type, );
+   rc = flash_bad_block(fd, mtd_type, blockstart);
if (rc < 0) /* block test failed */
return rc;
 
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/6] tools/env: factor out environment_end function

[Andreas Fenkart]

instead of adhoc computation of the environment end,
use a function with a proper name

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 32 +++-
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index d27f57e..90eb5fa 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -643,6 +643,18 @@ int fw_parse_script(char *fname, struct env_opts *opts)
return ret;
 }
 
+/**
+ * environment_end() - compute offset of first byte right after environemnt
+ * @dev - index of enviroment buffer
+ * Return:
+ *  device offset of first byte right after environemnt
+ */
+off_t environment_end(int dev)
+{
+   /* environment is block aligned */
+   return DEVOFFSET(dev) + ENVSECTORS(dev) * DEVESIZE(dev);
+}
+
 /*
  * Test for bad block on NAND, just returns 0 on NOR, on NAND:
  * 0   - block is good
@@ -683,7 +695,6 @@ static int flash_read_buf (int dev, int fd, void *buf, 
size_t count,
   0 on NOR */
size_t processed = 0;   /* progress counter */
size_t readlen = count; /* current read length */
-   off_t top_of_range; /* end of the last block we may use */
off_t block_seek;   /* offset inside the current block to the start
   of the data */
loff_t blockstart;  /* running start of the current block -
@@ -702,19 +713,11 @@ static int flash_read_buf (int dev, int fd, void *buf, 
size_t count,
 */
blocklen = DEVESIZE (dev);
 
-   /*
-* To calculate the top of the range, we have to use the
-* global DEVOFFSET (dev), which can be different from offset
-*/
-   top_of_range = ((DEVOFFSET(dev) / blocklen) +
-   ENVSECTORS (dev)) * blocklen;
-
/* Limit to one block for the first read */
if (readlen > blocklen - block_seek)
readlen = blocklen - block_seek;
} else {
blocklen = 0;
-   top_of_range = offset + count;
}
 
/* This only runs once on NOR flash */
@@ -723,7 +726,7 @@ static int flash_read_buf (int dev, int fd, void *buf, 
size_t count,
if (rc < 0) /* block test failed */
return -1;
 
-   if (blockstart + block_seek + readlen > top_of_range) {
+   if (blockstart + block_seek + readlen > environment_end(dev)) {
/* End of range is reached */
fprintf (stderr,
 "Too few good blocks within range\n");
@@ -783,7 +786,6 @@ static int flash_write_buf (int dev, int fd, void *buf, 
size_t count,
   below offset */
off_t block_seek;   /* offset inside the erase block to the start
   of the data */
-   off_t top_of_range; /* end of the last block we may use */
loff_t blockstart;  /* running start of the current block -
   MEMGETBADBLOCK needs 64 bits */
int rc;
@@ -793,7 +795,6 @@ static int flash_write_buf (int dev, int fd, void *buf, 
size_t count,
 */
if (mtd_type == MTD_ABSENT) {
blocklen = count;
-   top_of_range = offset + count;
erase_len = blocklen;
blockstart = offset;
block_seek = 0;
@@ -801,13 +802,10 @@ static int flash_write_buf (int dev, int fd, void *buf, 
size_t count,
} else {
blocklen = DEVESIZE(dev);
 
-   top_of_range = ((DEVOFFSET(dev) / blocklen) +
-   ENVSECTORS(dev)) * blocklen;
-
erase_offset = (offset / blocklen) * blocklen;
 
/* Maximum area we may use */
-   erase_len = top_of_range - erase_offset;
+   erase_len = environment_end(dev) - erase_offset;
 
blockstart = erase_offset;
/* Offset inside a block */
@@ -882,7 +880,7 @@ static int flash_write_buf (int dev, int fd, void *buf, 
size_t count,
if (rc < 0) /* block test failed */
return rc;
 
-   if (blockstart + erasesize > top_of_range) {
+   if (blockstart + erasesize > environment_end(dev)) {
fprintf (stderr, "End of range reached, aborting\n");
return -1;
}
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 0/6] tools/env: refactor flash read/write function

[Andreas Fenkart]

Take advantage of the environment being block aligned
block alignment is enforced since 183923d3e4

Andreas Fenkart (6):
  tools/env: factor out environment_end function
  tools/env: pass bad block offset by value
  tools/env: lookup dev_type directly from
flash_read_buf/flash_write_buf
  tools/env: flash_write_buf: enforce offset to be start of environment
  tools/env: flash_write_buf: remove block alignment calculations
  tools/env: flash_read_buf: enforce offset to be start of environment

 tools/env/fw_env.c | 121 -
 1 file changed, 45 insertions(+), 76 deletions(-)

-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/2] tools/env: return with error if redundant environments have unequal size

[Andreas Fenkart]

For double buffering to work, the target buffer must always be big
enough to hold all data. This can only be ensured if buffers are of
equal size, otherwise one must be smaller and we risk data loss
when copying from the bigger to the smaller buffer.

Reviewed-by: Simon Glass <s...@chromium.org>
Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index d2b167d..7cc7488 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -1423,10 +1423,9 @@ static int parse_config(struct env_opts *opts)
return rc;
 
if (ENVSIZE(0) != ENVSIZE(1)) {
-   ENVSIZE(0) = ENVSIZE(1) = min(ENVSIZE(0), ENVSIZE(1));
fprintf(stderr,
-   "Redundant environments have inequal size, set 
to 0x%08lx\n",
-   ENVSIZE(1));
+   "Redundant environments have unequal size");
+   return -1;
}
}
 
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/2] tools/env: soften warning about erase block alignment

[Andreas Fenkart]

addon 183923d3e
MMC/SATA have no erase blocks, only blocks. Hence the warning
about erase block alignment might be confusing in such environment.

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 7cc7488..d27f57e 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -1295,7 +1295,7 @@ static int check_device_config(int dev)
int fd, rc = 0;
 
if (DEVOFFSET(dev) % DEVESIZE(dev) != 0) {
-   fprintf(stderr, "Environment does not start on erase block 
boundary\n");
+   fprintf(stderr, "Environment does not start on (erase) block 
boundary\n");
errno = EINVAL;
return -1;
}
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 0/2] tools/env: minor fixes plus resend

[Andreas Fenkart]

The redundant environment patch was already submitted once,
but not applied due to a merge conflict.
The second patch is an addon to 183923d3e as suggested by Stefan Agner

Andreas Fenkart (2):
  tools/env: return with error if redundant environments have unequal
size
  tools/env: soften warning about erase block alignment

 tools/env/fw_env.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] tools/env: ensure environment starts at erase block boundary

[Andreas Fenkart]

56086921 added support for unaligned environments access.
U-boot itself does not support this:
- env_nand.c fails when using an unaligned offset. It produces an
  error in nand_erase_opts{drivers/mtd/nand/nand_util.c}
- in env_sf/env_flash the unused space at the end is preserved, but
  not in the beginning. block alignment is assumed
- env_sata/env_mmc aligns offset/length to the block size of the
  underlying device. data is silently redirected to the beginning of
  a block

There is seems no use case for unaligned environment. If there is
some useful data at the beginning of the the block (e.g. end of u-boot)
that would be very unsafe. If the redundant environments are hosted by
the same erase block then that invalidates the idea of double buffering.
It might be that unaligned access was allowed in the past, and that
people with legacy u-boot are trapped. But at the time of 56086921
it wasn't supported and due to reasons above I guess it was never
introduced.
I prefer to remove that (unused) feature in favor of simplicity

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 6b0dcaa..2ccc617 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -1294,6 +1294,18 @@ static int check_device_config(int dev)
struct stat st;
int fd, rc = 0;
 
+   if (DEVOFFSET(dev) % DEVESIZE(dev) != 0) {
+   fprintf(stderr, "Environment does not start on erase block 
boundary\n");
+   errno = EINVAL;
+   return -1;
+   }
+
+   if (ENVSIZE(i) > ENVSECTORS(i) * DEVESIZE(i)) {
+   fprintf(stderr, "Environment does not fit into available 
sectors\n");
+   errno = EINVAL;
+   return -1;
+   }
+
fd = open(DEVNAME(dev), O_RDONLY);
if (fd < 0) {
fprintf(stderr,
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [RFC: v2] tools/env: ensure environment starts at erase block boundary

[Andreas Fenkart]

56086921 added support for unaligned environments access.
U-boot itself does not support this:
- env_nand.c fails when using an unaligned offset. It produces an
  error in nand_erase_opts{drivers/mtd/nand/nand_util.c}
- in env_sf/env_flash the unused space at the end is preserved, but
  not in the beginning. block alignment is assumed
- env_sata/env_mmc aligns offset/length to the block size of the
  underlying device. data is silently redirected to the beginning of
  a block

There is seems no use case for unaligned environment. If there is
some useful data at the beginning of the the block (e.g. end of u-boot)
that would be very unsafe. If the redundant environments are hosted by
the same erase block then that invalidates the idea of double buffering.
It might be that unaligned access was allowed in the past, and that
people with legacy u-boot are trapped. But at the time of 56086921
it wasn't supported and due to reasons above I guess it was never
introduced.
I prefer to remove that (unused) feature in favor of simplicity

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 6b0dcaa..d2b167d 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -1294,6 +1294,18 @@ static int check_device_config(int dev)
struct stat st;
int fd, rc = 0;
 
+   if (DEVOFFSET(dev) % DEVESIZE(dev) != 0) {
+   fprintf(stderr, "Environment does not start on erase block 
boundary\n");
+   errno = EINVAL;
+   return -1;
+   }
+
+   if (ENVSIZE(dev) > ENVSECTORS(dev) * DEVESIZE(dev)) {
+   fprintf(stderr, "Environment does not fit into available 
sectors\n");
+   errno = EINVAL;
+   return -1;
+   }
+
fd = open(DEVNAME(dev), O_RDONLY);
if (fd < 0) {
fprintf(stderr,
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/2] tools/env: allow negative offsets

[Andreas Fenkart]

Hi,

2016-07-14 2:14 GMT+02:00 Stefan Agner :
> From: Stefan Agner 
>
> A negative value for the offset is treated as a backwards offset for
> from the end of the device/partition for block devices. This aligns
> the behavior of the config file with the syntax of CONFIG_ENV_OFFSET
> where the functionality has been introduced with
> commit 5c088ee841f9 ("env_mmc: allow negative CONFIG_ENV_OFFSET").
>
> Signed-off-by: Stefan Agner 
lgtm

> ---
>
>  tools/env/fw_env.c  | 39 ++-
>  tools/env/fw_env.config |  5 +
>  2 files changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> index 06d6865..be338a5b 100644
> --- a/tools/env/fw_env.c
> +++ b/tools/env/fw_env.c
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -51,7 +52,7 @@ struct env_opts default_opts = {
>
>  struct envdev_s {
> const char *devname;/* Device name */
> -   ulong devoff;   /* Device offset */
> +   long long devoff;   /* Device offset */
> ulong env_size; /* environment size */
> ulong erase_size;   /* device erase size */
> ulong env_sectors;  /* number of environment sectors */
> @@ -994,7 +995,7 @@ static int flash_write (int fd_current, int fd_target, 
> int dev_target)
> }
>
>  #ifdef DEBUG
> -   fprintf(stderr, "Writing new environment at 0x%lx on %s\n",
> +   fprintf(stderr, "Writing new environment at 0x%llx on %s\n",
> DEVOFFSET (dev_target), DEVNAME (dev_target));
>  #endif
>
> @@ -1010,7 +1011,7 @@ static int flash_write (int fd_current, int fd_target, 
> int dev_target)
> offsetof (struct env_image_redundant, flags);
>  #ifdef DEBUG
> fprintf(stderr,
> -   "Setting obsolete flag in environment at 0x%lx on 
> %s\n",
> +   "Setting obsolete flag in environment at 0x%llx on 
> %s\n",
> DEVOFFSET (dev_current), DEVNAME (dev_current));
>  #endif
> flash_flag_obsolete (dev_current, fd_current, offset);
> @@ -1335,7 +1336,27 @@ static int check_device_config(int dev)
> }
> DEVTYPE(dev) = mtdinfo.type;
> } else {
> +   uint64_t size;
> DEVTYPE(dev) = MTD_ABSENT;
> +
> +   /*
> +* Check for negative offsets, treat it as backwards offset
> +* from the end of the block device
> +*/
> +   if (DEVOFFSET(dev) < 0) {
> +   rc = ioctl(fd, BLKGETSIZE64, );
> +   if (rc < 0) {
> +   fprintf(stderr, "Could not get block device 
> size on %s\n",
> +   DEVNAME(dev));
> +   goto err;
> +   }
> +
> +   DEVOFFSET(dev) = DEVOFFSET(dev) + size;

I wonder if we need a check, that the redundant areas don't overlap.
But we can add that later


> +#ifdef DEBUG
> +   fprintf(stderr, "Calculated device offset 0x%llx on 
> %s\n",
> +   DEVOFFSET(dev), DEVNAME(dev));
> +#endif
> +   }
> }
>
>  err:
> @@ -1434,12 +1455,12 @@ static int get_config (char *fname)
> if (dump[0] == '#')
> continue;
>
> -   rc = sscanf (dump, "%ms %lx %lx %lx %lx",
> -,
> - (i),
> - (i),
> - (i),
> - (i));
> +   rc = sscanf(dump, "%ms %lli %lx %lx %lx",
> +   ,
> +   (i),
> +   (i),
> +   (i),
> +   (i));
>
> if (rc < 3)
> continue;
> diff --git a/tools/env/fw_env.config b/tools/env/fw_env.config
> index 6f216f9..f0f66aa 100644
> --- a/tools/env/fw_env.config
> +++ b/tools/env/fw_env.config
> @@ -4,6 +4,7 @@
>  # Notice, that the "Number of sectors" is not required on NOR and 
> SPI-dataflash.
>  # Futhermore, if the Flash sector size is ommitted, this value is assumed to
>  # be the same as the Environment size, which is valid for NOR and 
> SPI-dataflash
> +# Device offset must be prefixed with 0x to be parsed as a hexadecimal value.
>
>  # NOR example
>  # MTD device name  Device offset   Env. size   Flash sector size 
>   Number of sectors
> @@ -18,8 +19,12 @@
>  # NAND example
>  #/dev/mtd0 0x4000  0x4000  0x2   
>   2
>
> +# On a block device a negative offset is treated as a backwards offset from 
> the
> +# 

Re: [U-Boot] [PATCH 1/2] tools/env: complete environment device config early

[Andreas Fenkart]

2016-07-14 2:14 GMT+02:00 Stefan Agner <ste...@agner.ch>:
> From: Stefan Agner <stefan.ag...@toradex.com>
>
> Currently flash_read completes a crucial part of the environment
> device configuration, the device type (mtd_type). This is rather
> confusing as flash_io calls flash_read conditionally, and one might
> think flash_write, which also makes use of mtd_type, gets called
> before flash_read. But since flash_io is always called with O_RDONLY
> first, this is not actually the case in reality.
>
> However, it is much cleaner to complete and verify the config early
> in parse_config. This also prepares the code for further extension.
>
> Signed-off-by: Stefan Agner <stefan.ag...@toradex.com>
Reviewed-by: Andreas Fenkart
> ---
>
>  tools/env/fw_env.c | 110 
> +
>  1 file changed, 60 insertions(+), 50 deletions(-)
>
> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> index 692abda..06d6865 100644
> --- a/tools/env/fw_env.c
> +++ b/tools/env/fw_env.c
> @@ -1021,41 +1021,10 @@ static int flash_write (int fd_current, int 
> fd_target, int dev_target)
>
>  static int flash_read (int fd)
>  {
> -   struct mtd_info_user mtdinfo;
> -   struct stat st;
> int rc;
>
> -   rc = fstat(fd, );
> -   if (rc < 0) {
> -   fprintf(stderr, "Cannot stat the file %s\n",
> -   DEVNAME(dev_current));
> -   return -1;
> -   }
> -
> -   if (S_ISCHR(st.st_mode)) {
> -   rc = ioctl(fd, MEMGETINFO, );
> -   if (rc < 0) {
> -   fprintf(stderr, "Cannot get MTD information for %s\n",
> -   DEVNAME(dev_current));
> -   return -1;
> -   }
> -   if (mtdinfo.type != MTD_NORFLASH &&
> -   mtdinfo.type != MTD_NANDFLASH &&
> -   mtdinfo.type != MTD_DATAFLASH &&
> -   mtdinfo.type != MTD_UBIVOLUME) {
> -   fprintf (stderr, "Unsupported flash type %u on %s\n",
> -mtdinfo.type, DEVNAME(dev_current));
> -   return -1;
> -   }
> -   } else {
> -   memset(, 0, sizeof(mtdinfo));
> -   mtdinfo.type = MTD_ABSENT;
> -   }
> -
> -   DEVTYPE(dev_current) = mtdinfo.type;
> -
> rc = flash_read_buf(dev_current, fd, environment.image, CUR_ENVSIZE,
> -DEVOFFSET (dev_current), mtdinfo.type);
> +   DEVOFFSET(dev_current), DEVTYPE(dev_current));
> if (rc != CUR_ENVSIZE)
> return -1;
>
> @@ -1328,10 +1297,55 @@ int fw_env_open(struct env_opts *opts)
> return 0;
>  }
>
> +static int check_device_config(int dev)
> +{
> +   struct stat st;
> +   int fd, rc = 0;
> +
> +   fd = open(DEVNAME(dev), O_RDONLY);
> +   if (fd < 0) {
> +   fprintf(stderr,
> +   "Cannot open %s: %s\n",
> +   DEVNAME(dev), strerror(errno));
> +   return -1;
> +   }
> +
> +   rc = fstat(fd, );
> +   if (rc < 0) {
> +   fprintf(stderr, "Cannot stat the file %s\n",
> +   DEVNAME(dev));
> +   goto err;
> +   }
> +
> +   if (S_ISCHR(st.st_mode)) {
> +   struct mtd_info_user mtdinfo;
> +   rc = ioctl(fd, MEMGETINFO, );
> +   if (rc < 0) {
> +   fprintf(stderr, "Cannot get MTD information for %s\n",
> +   DEVNAME(dev));
> +   goto err;
> +   }
> +   if (mtdinfo.type != MTD_NORFLASH &&
> +   mtdinfo.type != MTD_NANDFLASH &&
> +   mtdinfo.type != MTD_DATAFLASH &&
> +   mtdinfo.type != MTD_UBIVOLUME) {
> +   fprintf(stderr, "Unsupported flash type %u on %s\n",
> +   mtdinfo.type, DEVNAME(dev));
> +   goto err;
> +   }
> +   DEVTYPE(dev) = mtdinfo.type;
> +   } else {
> +   DEVTYPE(dev) = MTD_ABSENT;
> +   }
> +
> +err:
> +   close(fd);
> +   return rc;
> +}
>
>  static int parse_config(struct env_opts *opts)
>  {
> -   struct stat st;
> +   int rc;
>
> if (!opts)
> opts = _opts;
> @@ -1375,25 +1389,21 @@ static int parse_con

Re: [U-Boot] [PATCH 2/4] tools/env: javadoc for fw_printenv, fw_getenv and fw_parse_script

[Andreas Fenkart]

Hi Simon,

I sent out a v2 of this series, you should have received by now. Some
comments in the context:

2016-07-09 16:39 GMT+02:00 Simon Glass <s...@chromium.org>:
> On 27 June 2016 at 16:06, Andreas Fenkart
> <andreas.fenk...@digitalstrom.com> wrote:

[snip]

>> +/**
>> + * fw_printenv - print one or several environment variables
>
> We normally add function brackets:

Hmm, I wonder what actually is used, since it's neither javadoc nor kernel-doc

http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html
http://www.mjmwired.net/kernel/Documentation/kernel-doc-nano-HOWTO.txt

For javadoc we would have to use @param  keyword instead of the
shorter @arg used in the patch. But kernel-doc does not support
@return/@returns, it requires a 'Return:' section.

I verified the markup using:
./scripts/kernel-doc -html -v tools/env/fw_env.h

This was just for convenience, since the script comes with the source.
I can switch to whatever format is preferred.

>fw_printenv() - print ...
>
>> + *
>> + * @argc  number of variables names to be printed, print all if 0
>> + * @argv  array of variable names to be printed
>> + * @value_only  do not repeat the variable name, only print the value,
>> + *  only one variable allowed with this option, argc must be 1
>> + * @opts  encryption key, configuration file, defaults are used if NULL
>
> @returns

$ grep -re  '\W@return\W' * | wc -l
1280

$ grep -re  '\W@returns\W' * | wc -l
12

javadoc uses @return

Is there a tool that processes the existing documentation markup,
probably it's not kernel-doc since it doesn't know the @return keyword

[snip]

/Andi
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 4/4] tools/env: reuse fw_getenv in fw_printenv function

[Andreas Fenkart]

Try to avoid adhoc iteration of the environment. Reuse fw_getenv
to find the variables that should be printed. Only use open-coded
iteration when printing all variables.
For backwards compatibility, keep emitting a newline when
printing with value_only.

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 44 +---
 1 file changed, 17 insertions(+), 27 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index f155c12..8b3a132 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -249,9 +249,14 @@ int parse_aes_key(char *key, uint8_t *bin_key)
  */
 int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts)
 {
-   char *env, *nxt;
int i, rc = 0;
 
+   if (value_only && argc != 1) {
+   fprintf(stderr,
+   "## Error: `-n' option requires exactly one 
argument\n");
+   return -1;
+   }
+
if (!opts)
opts = _opts;
 
@@ -259,6 +264,7 @@ int fw_printenv(int argc, char *argv[], int value_only, 
struct env_opts *opts)
return -1;
 
if (argc == 0) {/* Print all env variables  */
+   char *env, *nxt;
for (env = environment.data; *env; env = nxt + 1) {
for (nxt = env; *nxt; ++nxt) {
if (nxt >= [ENV_SIZE]) {
@@ -273,39 +279,23 @@ int fw_printenv(int argc, char *argv[], int value_only, 
struct env_opts *opts)
return 0;
}
 
-   if (value_only && argc != 1) {
-   fprintf(stderr,
-   "## Error: `-n' option requires exactly one 
argument\n");
-   return -1;
-   }
-
-   for (i = 0; i < argc; ++i) {/* print single env variables   */
+   for (i = 0; i < argc; ++i) {/* print a subset of env variables */
char *name = argv[i];
char *val = NULL;
 
-   for (env = environment.data; *env; env = nxt + 1) {
-
-   for (nxt = env; *nxt; ++nxt) {
-   if (nxt >= [ENV_SIZE]) {
-   fprintf (stderr, "## Error: "
-   "environment not terminated\n");
-   return -1;
-   }
-   }
-   val = envmatch (name, env);
-   if (val) {
-   if (!value_only) {
-   fputs (name, stdout);
-   putc ('=', stdout);
-   }
-   puts (val);
-   break;
-   }
-   }
+   val = fw_getenv(name);
if (!val) {
fprintf (stderr, "## Error: \"%s\" not defined\n", 
name);
rc = -1;
+   continue;
}
+
+   if (value_only) {
+   puts(val);
+   break;
+   }
+
+   printf("%s=%s\n", name, val);
}
 
return rc;
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 3/4] tools/env: move envmatch further up in file to avoid forward declarations

[Andreas Fenkart]

forward declaration not needed when re-ordered

Reviewed-by: Simon Glass <s...@chromium.org>
Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 38 ++
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index fe479e9..f155c12 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -121,7 +121,6 @@ static unsigned char obsolete_flag = 0;
 #include 
 
 static int flash_io (int mode);
-static char *envmatch (char * s1, char * s2);
 static int parse_config(struct env_opts *opts);
 
 #if defined(CONFIG_FILE)
@@ -147,6 +146,24 @@ static char *skip_blanks(char *s)
 }
 
 /*
+ * s1 is either a simple 'name', or a 'name=value' pair.
+ * s2 is a 'name=value' pair.
+ * If the names match, return the value of s2, else NULL.
+ */
+static char *envmatch(char *s1, char *s2)
+{
+   if (s1 == NULL || s2 == NULL)
+   return NULL;
+
+   while (*s1 == *s2++)
+   if (*s1++ == '=')
+   return s2;
+   if (*s1 == '\0' && *(s2 - 1) == '=')
+   return s2;
+   return NULL;
+}
+
+/**
  * Search the environment for a variable.
  * Return the value, if found, or NULL, if not found.
  */
@@ -1121,25 +1138,6 @@ exit:
 }
 
 /*
- * s1 is either a simple 'name', or a 'name=value' pair.
- * s2 is a 'name=value' pair.
- * If the names match, return the value of s2, else NULL.
- */
-
-static char *envmatch (char * s1, char * s2)
-{
-   if (s1 == NULL || s2 == NULL)
-   return NULL;
-
-   while (*s1 == *s2++)
-   if (*s1++ == '=')
-   return s2;
-   if (*s1 == '\0' && *(s2 - 1) == '=')
-   return s2;
-   return NULL;
-}
-
-/*
  * Prevent confusion if running from erased flash memory
  */
 int fw_env_open(struct env_opts *opts)
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 0/4] tools/env: minor refactorings

[Andreas Fenkart]

various patches, see individual patch descriptions

Andreas Fenkart (4):
  tools/env: return with error if redundant environments have unequal
size
  tools/env: kernel-doc for fw_printenv, fw_getenv and fw_parse_script
  tools/env: move envmatch further up in file to avoid forward
declarations
  tools/env: reuse fw_getenv in fw_printenv function

 tools/env/fw_env.c | 107 ++---
 tools/env/fw_env.h | 115 -
 2 files changed, 162 insertions(+), 60 deletions(-)

-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 1/4] tools/env: return with error if redundant environments have unequal size

[Andreas Fenkart]

For double buffering to work, the target buffer must always be big
enough to hold all data. This can only be ensured if buffers are of
equal size, otherwise one must be smaller and we risk data loss
when copying from the bigger to the smaller buffer.

Reviewed-by: Simon Glass <s...@chromium.org>
Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 692abda..b1c8217 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -1382,18 +1382,19 @@ static int parse_config(struct env_opts *opts)
return -1;
}
 
-   if (HaveRedundEnv && stat (DEVNAME (1), )) {
-   fprintf (stderr,
-   "Cannot access MTD device %s: %s\n",
-   DEVNAME (1), strerror (errno));
-   return -1;
-   }
+   if (HaveRedundEnv) {
+   if (stat(DEVNAME(1), )) {
+   fprintf(stderr,
+   "Cannot access MTD device %s: %s\n",
+   DEVNAME(1), strerror(errno));
+   return -1;
+   }
 
-   if (HaveRedundEnv && ENVSIZE(0) != ENVSIZE(1)) {
-   ENVSIZE(0) = ENVSIZE(1) = min(ENVSIZE(0), ENVSIZE(1));
-   fprintf(stderr,
-   "Redundant environments have inequal size, set to 
0x%08lx\n",
-   ENVSIZE(1));
+   if (ENVSIZE(0) != ENVSIZE(1)) {
+   fprintf(stderr,
+   "Redundant environments have unequal size");
+   return -1;
+   }
}
 
usable_envsize = CUR_ENVSIZE - sizeof(uint32_t);
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 2/4] tools/env: kernel-doc for fw_printenv, fw_getenv and fw_parse_script

[Andreas Fenkart]

there are two groups of functions:
- application ready tools: fw_setenv/fw_getenv/fw_parse_script
these are used, when creating a single binary containing multiple
tools (busybox like)
- file access like: open/read/write/close
above functions are implemented on top of these. applications
can use those to modify several variables without creating a
temporary batch script file
tested with "./scripts/kernel-doc -html -v tools/env/fw_env.h"

Reviewed-by: Simon Glass <s...@chromium.org>
Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c |   2 +-
 tools/env/fw_env.h | 115 -
 2 files changed, 115 insertions(+), 2 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index b1c8217..fe479e9 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -483,7 +483,7 @@ int fw_setenv(int argc, char *argv[], struct env_opts *opts)
valc = argc - 1;
 
if (env_flags_validate_env_set_params(name, valv, valc) < 0)
-   return 1;
+   return -1;
 
len = 0;
for (i = 0; i < valc; ++i) {
diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
index dac964d..436eca9 100644
--- a/tools/env/fw_env.h
+++ b/tools/env/fw_env.h
@@ -67,12 +67,125 @@ struct env_opts {
 
 int parse_aes_key(char *key, uint8_t *bin_key);
 
+/**
+ * fw_printenv() - print one or several environment variables
+ *
+ * @argc: number of variables names to be printed, prints all if 0
+ * @argv: array of variable names to be printed, if argc != 0
+ * @value_only: do not repeat the variable name, print the bare value,
+ *  only one variable allowed with this option, argc must be 1
+ * @opts: encryption key, configuration file, defaults are used if NULL
+ *
+ * Description:
+ *  Uses fw_env_open, fw_getenv
+ *
+ * Return:
+ *  0 on success, -1 on failure (modifies errno)
+ */
 int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts);
-char *fw_getenv(char *name);
+
+/**
+ * fw_setenv() - adds or removes one variable to the environment
+ *
+ * @argc: number of strings in argv, argv[0] is variable name,
+ *  argc==1 means erase variable, argc > 1 means add a variable
+ * @argv: argv[0] is variable name, argv[1..argc-1] are concatenated separated
+ *   by single blank and set as the new value of the variable
+ * @opts: how to retrieve environment from flash, defaults are used if NULL
+ *
+ * Description:
+ *  Uses fw_env_open, fw_env_write, fw_env_close
+ *
+ * Return:
+ *  0 on success, -1 on failure (modifies errno)
+ *
+ * ERRORS:
+ *  EROFS - some variables ("ethaddr", "serial#") cannot be modified
+ */
 int fw_setenv(int argc, char *argv[], struct env_opts *opts);
+
+/**
+ * fw_parse_script() - adds or removes multiple variables with a batch script
+ *
+ * @fname: batch script file name
+ * @opts: encryption key, configuration file, defaults are used if NULL
+ *
+ * Description:
+ *  Uses fw_env_open, fw_env_write, fw_env_close
+ *
+ * Return:
+ *  0 success, -1 on failure (modifies errno)
+ *
+ * Script Syntax:
+ *
+ *  key [ [space]+ value]
+ *
+ *  lines starting with '#' treated as comment
+ *
+ *  A variable without value will be deleted. Any number of spaces are allowed
+ *  between key and value. The value starts with the first non-space character
+ *  and ends with newline. No comments allowed on these lines.  Spaces inside
+ *  the value are preserved verbatim.
+ *
+ * Script Example:
+ *
+ *  netdev eth0
+ *
+ *  kernel_addr40
+ *
+ *  foospaces   are copied verbatim
+ *
+ *  # delete variable bar
+ *
+ *  bar
+ */
 int fw_parse_script(char *fname, struct env_opts *opts);
+
+
+/**
+ * fw_env_open() - read enviroment from flash into RAM cache
+ *
+ * @opts: encryption key, configuration file, defaults are used if NULL
+ *
+ * Return:
+ *  0 on success, -1 on failure (modifies errno)
+ */
 int fw_env_open(struct env_opts *opts);
+
+/**
+ * fw_getenv() - lookup variable in the RAM cache
+ *
+ * @name: variable to be searched
+ * Return:
+ *  pointer to start of value, NULL if not found
+ */
+char *fw_getenv(char *name);
+
+/**
+ * fw_env_write() - modify a variable held in the RAM cache
+ *
+ * @name: variable
+ * @value: delete variable if NULL, otherwise create or overwrite the variable
+ *
+ * This is called in sequence to update the environment in RAM without updating
+ * the copy in flash after each set
+ *
+ * Return:
+ *  0 on success, -1 on failure (modifies errno)
+ *
+ * ERRORS:
+ *  EROFS - some variables ("ethaddr", "serial#") cannot be modified
+ */
 int fw_env_write(char *name, char *value);
+
+/**
+ * fw_env_close - write the environment from RAM cache back to flash
+ *
+ * @opts: encryption key, configuration file, defaults are used if NULL
+ *
+ * Return:
+ *  0 on success, -1 on failure (modifies errno)
+ */
 int fw_env_close(struct env

[U-Boot] [PATCH 2/4] tools/env: javadoc for fw_printenv, fw_getenv and fw_parse_script

[Andreas Fenkart]

there are two groups of functions:
- application ready tools: fw_setenv/fw_getenv/fw_parse_script
these are used, when creating a single binary containing multiple
tools (busybox like)
- file access like: open/read/write/close
above functions are implemented on top of these. applications
can use those to modify several variables without creating a
temporary batch script file

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.h | 75 +-
 1 file changed, 74 insertions(+), 1 deletion(-)

diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
index dac964d..8bf74f3 100644
--- a/tools/env/fw_env.h
+++ b/tools/env/fw_env.h
@@ -67,12 +67,85 @@ struct env_opts {
 
 int parse_aes_key(char *key, uint8_t *bin_key);
 
+/**
+ * fw_printenv - print one or several environment variables
+ *
+ * @argc  number of variables names to be printed, print all if 0
+ * @argv  array of variable names to be printed
+ * @value_only  do not repeat the variable name, only print the value,
+ *  only one variable allowed with this option, argc must be 1
+ * @opts  encryption key, configuration file, defaults are used if NULL
+ */
 int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts);
-char *fw_getenv(char *name);
+
+/**
+ * fw_setenv - adds or removes one variable from the environment
+ *
+ * @argc  number of strings in argv, argv[0] is variable name,
+ *  argc==1 means erase variable, argc > 1 means add a variable
+ * @argv  argv[0] is variable name, argv[1..argc-1] are concatenated separated
+ *   by single blank and set as the new value of the variable
+ * @opts  how to retrieve environment from flash, defaults are used if NULL
+ * @ret   EROFS if variable must not be changed (ethaddr, serial#)
+ */
 int fw_setenv(int argc, char *argv[], struct env_opts *opts);
+
+/**
+ * fw_parse_script - add/remove multiple variables with a batch script
+ *
+ * @fname  batch script file name
+ * @opts  encryption key, configuration file, defaults are used if NULL
+ *
+ * Script Syntax:
+ *   key [[space]+ value]
+ *   lines starting with '#' treated as comment
+ *
+ *   A variable without value will be deleted. Any number of spaces are allowed
+ *   between key and value. The value starts with the first non-space character
+ *   and ends with newline. No comments allowed on these lines.  Spaces inside
+ *   the value are preserved verbatim.
+ *
+ * Script Example:"
+ *   netdev eth0
+ *   kernel_addr40
+ *   foospaces   are copied verbatim
+ *   # delete variable bar
+ *   bar\n"
+ */
 int fw_parse_script(char *fname, struct env_opts *opts);
+
+
+/**
+ * fw_env_open - read enviroment from flash and cache it in RAM
+ *
+ * @opts  encryption key, configuration file, defaults are used if NULL
+ */
 int fw_env_open(struct env_opts *opts);
+
+/**
+ * fw_getenv - lookup variable in RAM cache
+ *
+ * @name  variable to be searched
+ * @ret   NULL if not found
+ */
+char *fw_getenv(char *name);
+
+/**
+ * fw_env_write - set/clear a single variable in the RAM cache
+ *
+ * This is called in sequence to update the environment in RAM without updating
+ * the copy in flash after each set
+ *
+ * @name  variable
+ * @value delete variable if NULL, otherwise create or overwrite the variable
+ */
 int fw_env_write(char *name, char *value);
+
+/**
+ * fw_env_close - writes environment from RAM back to flash
+ *
+ * @opts  encryption key, configuration file, defaults are used if NULL
+ */
 int fw_env_close(struct env_opts *opts);
 
 unsigned long crc32(unsigned long, const unsigned char *, unsigned);
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/4] tools/env: return with error if redundant environments have unequal size

[Andreas Fenkart]

From: Andreas Fenkart <afenk...@gmail.com>

For double buffering to work, the target buffer must always be big
enough to hold all data. This can only be ensured if buffers are of
equal size, otherwise one must be smaller and we risk data loss
when copying from the bigger to the smaller buffer.

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 692abda..b1c8217 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -1382,18 +1382,19 @@ static int parse_config(struct env_opts *opts)
return -1;
}
 
-   if (HaveRedundEnv && stat (DEVNAME (1), )) {
-   fprintf (stderr,
-   "Cannot access MTD device %s: %s\n",
-   DEVNAME (1), strerror (errno));
-   return -1;
-   }
+   if (HaveRedundEnv) {
+   if (stat(DEVNAME(1), )) {
+   fprintf(stderr,
+   "Cannot access MTD device %s: %s\n",
+   DEVNAME(1), strerror(errno));
+   return -1;
+   }
 
-   if (HaveRedundEnv && ENVSIZE(0) != ENVSIZE(1)) {
-   ENVSIZE(0) = ENVSIZE(1) = min(ENVSIZE(0), ENVSIZE(1));
-   fprintf(stderr,
-   "Redundant environments have inequal size, set to 
0x%08lx\n",
-   ENVSIZE(1));
+   if (ENVSIZE(0) != ENVSIZE(1)) {
+   fprintf(stderr,
+   "Redundant environments have unequal size");
+   return -1;
+   }
}
 
usable_envsize = CUR_ENVSIZE - sizeof(uint32_t);
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 4/4] tools/env: reuse fw_getenv in fw_printenv function

[Andreas Fenkart]

Try to avoid adhoc iteration of the environment. Reuse fw_getenv
to find the variables that should be printed. Only use open-coded
iteration when printing all variables.
For backwards compatibility, keep emitting a newline when
printing with noheader.

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 44 +---
 1 file changed, 17 insertions(+), 27 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index ca839d1..9733950 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -249,9 +249,14 @@ int parse_aes_key(char *key, uint8_t *bin_key)
  */
 int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts)
 {
-   char *env, *nxt;
int i, rc = 0;
 
+   if (value_only && argc != 1) {
+   fprintf(stderr,
+   "## Error: `-n' option requires exactly one 
argument\n");
+   return -1;
+   }
+
if (!opts)
opts = _opts;
 
@@ -259,6 +264,7 @@ int fw_printenv(int argc, char *argv[], int value_only, 
struct env_opts *opts)
return -1;
 
if (argc == 0) {/* Print all env variables  */
+   char *env, *nxt;
for (env = environment.data; *env; env = nxt + 1) {
for (nxt = env; *nxt; ++nxt) {
if (nxt >= [ENV_SIZE]) {
@@ -273,39 +279,23 @@ int fw_printenv(int argc, char *argv[], int value_only, 
struct env_opts *opts)
return 0;
}
 
-   if (value_only && argc != 1) {
-   fprintf(stderr,
-   "## Error: `-n' option requires exactly one 
argument\n");
-   return -1;
-   }
-
-   for (i = 0; i < argc; ++i) {/* print single env variables   */
+   for (i = 0; i < argc; ++i) {/* print a subset of env variables */
char *name = argv[i];
char *val = NULL;
 
-   for (env = environment.data; *env; env = nxt + 1) {
-
-   for (nxt = env; *nxt; ++nxt) {
-   if (nxt >= [ENV_SIZE]) {
-   fprintf (stderr, "## Error: "
-   "environment not terminated\n");
-   return -1;
-   }
-   }
-   val = envmatch (name, env);
-   if (val) {
-   if (!value_only) {
-   fputs (name, stdout);
-   putc ('=', stdout);
-   }
-   puts (val);
-   break;
-   }
-   }
+   val = fw_getenv(name);
if (!val) {
fprintf (stderr, "## Error: \"%s\" not defined\n", 
name);
rc = -1;
+   continue;
}
+
+   if (value_only) {
+   puts(val);
+   break;
+   }
+
+   printf("%s=%s\n", name, val);
}
 
return rc;
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 3/4] tools/env: move envmatch further up in file to avoid forward declarations

[Andreas Fenkart]

From: Andreas Fenkart <afenk...@gmail.com>

forward declaration not needed when re-ordered

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 38 ++
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index b1c8217..ca839d1 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -121,7 +121,6 @@ static unsigned char obsolete_flag = 0;
 #include 
 
 static int flash_io (int mode);
-static char *envmatch (char * s1, char * s2);
 static int parse_config(struct env_opts *opts);
 
 #if defined(CONFIG_FILE)
@@ -147,6 +146,24 @@ static char *skip_blanks(char *s)
 }
 
 /*
+ * s1 is either a simple 'name', or a 'name=value' pair.
+ * s2 is a 'name=value' pair.
+ * If the names match, return the value of s2, else NULL.
+ */
+static char *envmatch(char *s1, char *s2)
+{
+   if (s1 == NULL || s2 == NULL)
+   return NULL;
+
+   while (*s1 == *s2++)
+   if (*s1++ == '=')
+   return s2;
+   if (*s1 == '\0' && *(s2 - 1) == '=')
+   return s2;
+   return NULL;
+}
+
+/**
  * Search the environment for a variable.
  * Return the value, if found, or NULL, if not found.
  */
@@ -1121,25 +1138,6 @@ exit:
 }
 
 /*
- * s1 is either a simple 'name', or a 'name=value' pair.
- * s2 is a 'name=value' pair.
- * If the names match, return the value of s2, else NULL.
- */
-
-static char *envmatch (char * s1, char * s2)
-{
-   if (s1 == NULL || s2 == NULL)
-   return NULL;
-
-   while (*s1 == *s2++)
-   if (*s1++ == '=')
-   return s2;
-   if (*s1 == '\0' && *(s2 - 1) == '=')
-   return s2;
-   return NULL;
-}
-
-/*
  * Prevent confusion if running from erased flash memory
  */
 int fw_env_open(struct env_opts *opts)
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 0/4] tools/env: minor refactorings

[Andreas Fenkart]

various patches, see individual patch descriptions

Andreas Fenkart (4):
  tools/env: return with error if redundant environments have unequal
size
  tools/env: javadoc for fw_printenv, fw_getenv and fw_parse_script
  tools/env: move envmatch further up in file to avoid forward
declarations
  tools/env: reuse fw_getenv in fw_printenv function

 tools/env/fw_env.c | 105 -
 tools/env/fw_env.h |  75 +-
 2 files changed, 121 insertions(+), 59 deletions(-)

-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] fw_env: Make env flash lock optional

[Andreas Fenkart]

Hi,

I faced the same problem a while back, thanks for taking a look at this.
Does it have to be compiled in? Why not make it a command line option,
evtl. making the default a compile time option.

/Andi

2016-06-02 10:27 GMT+02:00 Dirk Eibach :
> Hi Anatolij,
>
> 2016-06-02 10:14 GMT+02:00 Anatolij Gustschin :
> ...
>> when MEMLOCK ist broken on a platform and disabled by your patch,
>> shouldn't MEMUNLOCK be isolated as well?
>
> There are  cornercases where lock/unlock works properly in the kernel
> but not in u-boot. So we might have an environment that was locked by
> u-boot and are able to unlock it with fw_env.
>
> Cheers
> Dirk
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] tools/env: check that redundant environments have equal size

[Andreas Fenkart]

Hi Tom

I thought there was something wrong with my april patches, so I
started reworking them. This patch is part of the rework. I broke it
out of the series to make it stand out of the other patches since it
might break behaviour. You can ignore it, it's part of the v2 series.
I will rebase my cleaned up patches when I have time. One problem
though is that the env-tools on master now have a compile error. I
will fix that and send out a patch soon...

Andreas

2016-05-30 20:01 GMT+02:00 Tom Rini <tr...@konsulko.com>:
> On Mon, May 30, 2016 at 01:59:51PM -0400, Tom Rini wrote:
>> On Thu, May 19, 2016 at 12:43:51PM +0200, Andreas Fenkart wrote:
>>
>> > For double buffering to work, the target buffer must always be big
>> > enough to hold all data. This can only be ensured if buffers are of
>> > equal size, otherwise one must be smaller and we risk data loss
>> > when copying from the bigger to the smaller.
>> >
>> > Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
>>
>> Applied to u-boot/master, thanks!
>
> Actually, wait, no, this one isn't, with your other series now applied,
> this doesn't apply cleanly, thanks!
>
> --
> Tom
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] tools/env: allow to pass NULL for environment options

[Andreas Fenkart]

If users of the library are happy with the default, e.g. config file
name. They can pass NULL as the opts pointer. This simplifies the
transition of existing library users.
FIXES a compile error. since common_args has been removed by
a previous patch

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 27 ---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 52e0bec..692abda 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -35,6 +35,12 @@
 
 #include "fw_env.h"
 
+struct env_opts default_opts = {
+#ifdef CONFIG_FILE
+   .config_file = CONFIG_FILE
+#endif
+};
+
 #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
 
 #define min(x, y) ({   \
@@ -229,6 +235,9 @@ int fw_printenv(int argc, char *argv[], int value_only, 
struct env_opts *opts)
char *env, *nxt;
int i, rc = 0;
 
+   if (!opts)
+   opts = _opts;
+
if (fw_env_open(opts))
return -1;
 
@@ -289,6 +298,9 @@ int fw_env_close(struct env_opts *opts)
 {
int ret;
 
+   if (!opts)
+   opts = _opts;
+
if (opts->aes_flag) {
ret = env_aes_cbc_crypt(environment.data, 1,
opts->aes_key);
@@ -452,6 +464,9 @@ int fw_setenv(int argc, char *argv[], struct env_opts *opts)
char *value = NULL;
int valc;
 
+   if (!opts)
+   opts = _opts;
+
if (argc < 1) {
fprintf(stderr, "## Error: variable name missing\n");
errno = EINVAL;
@@ -524,6 +539,9 @@ int fw_parse_script(char *fname, struct env_opts *opts)
int len;
int ret = 0;
 
+   if (!opts)
+   opts = _opts;
+
if (fw_env_open(opts)) {
fprintf(stderr, "Error: environment not initialized\n");
return -1;
@@ -1139,6 +1157,9 @@ int fw_env_open(struct env_opts *opts)
struct env_image_single *single;
struct env_image_redundant *redundant;
 
+   if (!opts)
+   opts = _opts;
+
if (parse_config(opts)) /* should fill envdevices */
return -1;
 
@@ -1312,10 +1333,10 @@ static int parse_config(struct env_opts *opts)
 {
struct stat st;
 
-#if defined(CONFIG_FILE)
-   if (!common_args.config_file)
-   common_args.config_file = CONFIG_FILE;
+   if (!opts)
+   opts = _opts;
 
+#if defined(CONFIG_FILE)
/* Fills in DEVNAME(), ENVSIZE(), DEVESIZE(). Or don't. */
if (get_config(opts->config_file)) {
fprintf(stderr, "Cannot parse config file '%s': %m\n",
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 4/5] tools/env: compute size of usable area only once

[Andreas Fenkart]

for double buffering to work, redundant buffers must have equal size

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 32 ++--
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 1420855..15df5ad 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -75,7 +75,8 @@ static int dev_current;
 
 #define CUR_ENVSIZE ENVSIZE(dev_current)
 
-#define ENV_SIZE  getenvsize()
+static unsigned long usable_envsize;
+#define ENV_SIZE  usable_envsize
 
 struct env_image_single {
uint32_tcrc;/* CRC32 over data bytes*/
@@ -124,18 +125,6 @@ static int parse_config (void);
 #if defined(CONFIG_FILE)
 static int get_config (char *);
 #endif
-static inline ulong getenvsize (void)
-{
-   ulong rc = CUR_ENVSIZE - sizeof(uint32_t);
-
-   if (HaveRedundEnv)
-   rc -= sizeof (char);
-
-   if (common_args.aes_flag)
-   rc &= ~(AES_KEY_LENGTH - 1);
-
-   return rc;
-}
 
 static char *skip_chars(char *s)
 {
@@ -953,7 +942,7 @@ static int flash_flag_obsolete (int dev, int fd, off_t 
offset)
 static int env_aes_cbc_crypt(char *payload, const int enc, uint8_t *key)
 {
uint8_t *data = (uint8_t *)payload;
-   const int len = getenvsize();
+   const int len = usable_envsize;
uint8_t key_exp[AES_EXPAND_KEY_LENGTH];
uint32_t aes_blocks;
 
@@ -1379,6 +1368,21 @@ static int parse_config ()
DEVNAME (1), strerror (errno));
return -1;
}
+
+   if (HaveRedundEnv && ENVSIZE(0) != ENVSIZE(1)) {
+   ENVSIZE(0) = ENVSIZE(1) = min(ENVSIZE(0), ENVSIZE(1));
+   fprintf(stderr,
+   "Redundant environments have inequal size, set to 
0x%08lx\n",
+   ENVSIZE(1));
+   }
+
+   usable_envsize = CUR_ENVSIZE - sizeof(uint32_t);
+   if (HaveRedundEnv)
+   usable_envsize -= sizeof(char);
+
+   if (common_args.aes_flag)
+   usable_envsize &= ~(AES_KEY_LENGTH - 1);
+
return 0;
 }
 
-- 
2.8.0.rc3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 3/5] tools/env: fw_printenv pass value_only as argument

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c  | 6 +++---
 tools/env/fw_env.h  | 4 ++--
 tools/env/fw_env_main.c | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index e5b2e8f..1420855 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -239,7 +239,7 @@ int parse_aes_key(char *key, uint8_t *bin_key)
  * Print the current definition of one, or more, or all
  * environment variables
  */
-int fw_printenv (int argc, char *argv[])
+int fw_printenv(int argc, char *argv[], int value_only)
 {
char *env, *nxt;
int i, rc = 0;
@@ -262,7 +262,7 @@ int fw_printenv (int argc, char *argv[])
return 0;
}
 
-   if (printenv_args.name_suppress && argc != 1) {
+   if (value_only && argc != 1) {
fprintf(stderr,
"## Error: `-n' option requires exactly one 
argument\n");
return -1;
@@ -283,7 +283,7 @@ int fw_printenv (int argc, char *argv[])
}
val = envmatch (name, env);
if (val) {
-   if (!printenv_args.name_suppress) {
+   if (!value_only) {
fputs (name, stdout);
putc ('=', stdout);
}
diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
index 7345922..d4daeea 100644
--- a/tools/env/fw_env.h
+++ b/tools/env/fw_env.h
@@ -67,7 +67,7 @@ struct common_args {
 extern struct common_args common_args;
 
 struct printenv_args {
-   int name_suppress;
+   int value_only;
 };
 extern struct printenv_args printenv_args;
 
@@ -78,7 +78,7 @@ extern struct setenv_args setenv_args;
 
 int parse_aes_key(char *key, uint8_t *bin_key);
 
-int   fw_printenv(int argc, char *argv[]);
+int fw_printenv(int argc, char *argv[], int value_only);
 char *fw_getenv(char *name);
 int fw_setenv(int argc, char *argv[]);
 int fw_parse_script(char *fname);
diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index 3706d8f..2a45a0d 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -151,7 +151,7 @@ int parse_printenv_args(int argc, char *argv[])
   EOF) {
switch (c) {
case 'n':
-   printenv_args.name_suppress = 1;
+   printenv_args.value_only = 1;
break;
case 'a':
case 'c':
@@ -240,7 +240,7 @@ int main(int argc, char *argv[])
}
 
if (do_printenv) {
-   if (fw_printenv(argc, argv) != 0)
+   if (fw_printenv(argc, argv, printenv_args.value_only))
retval = EXIT_FAILURE;
} else {
if (!setenv_args.script_file) {
-- 
2.8.0.rc3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 0/5] tools/env: pass arguments as parameters not global structure

[Andreas Fenkart]

u-boot tools can be built as a library (libubootenv.a). Passing arguments
to the library using global structures is a bad interface.

v2:
- fix env_aes_cbc_crypt prototype
- check opts pointer for not NULL

Andreas Fenkart (5):
  tools/env: pass key as argument to env_aes_cbc_crypt
  tools/env: remove 'extern' from function prototype in fw_env.h
  tools/env: fw_printenv pass value_only as argument
  tools/env: compute size of usable area only once
  tools/env: replace global variables by parameter passing

 tools/env/fw_env.c  | 111 
 tools/env/fw_env.h  |  31 +-
 tools/env/fw_env_main.c |  28 +++-
 3 files changed, 94 insertions(+), 76 deletions(-)

-- 
2.8.0.rc3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 1/5] tools/env: pass key as argument to env_aes_cbc_crypt

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 1420ac5..e5b2e8f 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -106,7 +106,7 @@ static struct environment environment = {
.flag_scheme = FLAG_NONE,
 };
 
-static int env_aes_cbc_crypt(char *data, const int enc);
+static int env_aes_cbc_crypt(char *data, const int enc, uint8_t *key);
 
 static int HaveRedundEnv = 0;
 
@@ -304,7 +304,8 @@ int fw_env_close(void)
 {
int ret;
if (common_args.aes_flag) {
-   ret = env_aes_cbc_crypt(environment.data, 1);
+   ret = env_aes_cbc_crypt(environment.data, 1,
+   common_args.aes_key);
if (ret) {
fprintf(stderr,
"Error: can't encrypt env for flash\n");
@@ -949,7 +950,7 @@ static int flash_flag_obsolete (int dev, int fd, off_t 
offset)
 }
 
 /* Encrypt or decrypt the environment before writing or reading it. */
-static int env_aes_cbc_crypt(char *payload, const int enc)
+static int env_aes_cbc_crypt(char *payload, const int enc, uint8_t *key)
 {
uint8_t *data = (uint8_t *)payload;
const int len = getenvsize();
@@ -957,7 +958,7 @@ static int env_aes_cbc_crypt(char *payload, const int enc)
uint32_t aes_blocks;
 
/* First we expand the key. */
-   aes_expand_key(common_args.aes_key, key_exp);
+   aes_expand_key(key, key_exp);
 
/* Calculate the number of AES blocks to encrypt. */
aes_blocks = DIV_ROUND_UP(len, AES_KEY_LENGTH);
@@ -1186,7 +1187,8 @@ int fw_env_open(void)
crc0 = crc32 (0, (uint8_t *) environment.data, ENV_SIZE);
 
if (common_args.aes_flag) {
-   ret = env_aes_cbc_crypt(environment.data, 0);
+   ret = env_aes_cbc_crypt(environment.data, 0,
+   common_args.aes_key);
if (ret)
return ret;
}
@@ -1243,7 +1245,8 @@ int fw_env_open(void)
crc1 = crc32 (0, (uint8_t *) redundant->data, ENV_SIZE);
 
if (common_args.aes_flag) {
-   ret = env_aes_cbc_crypt(redundant->data, 0);
+   ret = env_aes_cbc_crypt(redundant->data, 0,
+   common_args.aes_key);
if (ret)
return ret;
}
-- 
2.8.0.rc3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 2/5] tools/env: remove 'extern' from function prototype in fw_env.h

[Andreas Fenkart]

checkpatch complains about in succeding patch. Prefer to fix all
declarations in a dedicated patch.

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.h | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
index 57149e7..7345922 100644
--- a/tools/env/fw_env.h
+++ b/tools/env/fw_env.h
@@ -78,12 +78,12 @@ extern struct setenv_args setenv_args;
 
 int parse_aes_key(char *key, uint8_t *bin_key);
 
-extern int   fw_printenv(int argc, char *argv[]);
-extern char *fw_getenv  (char *name);
-extern int fw_setenv  (int argc, char *argv[]);
-extern int fw_parse_script(char *fname);
-extern int fw_env_open(void);
-extern int fw_env_write(char *name, char *value);
-extern int fw_env_close(void);
+int   fw_printenv(int argc, char *argv[]);
+char *fw_getenv(char *name);
+int fw_setenv(int argc, char *argv[]);
+int fw_parse_script(char *fname);
+int fw_env_open(void);
+int fw_env_write(char *name, char *value);
+int fw_env_close(void);
 
-extern unsignedlong  crc32  (unsigned long, const unsigned char *, 
unsigned);
+unsigned long crc32(unsigned long, const unsigned char *, unsigned);
-- 
2.8.0.rc3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/5] tools/env: remove 'extern' from function prototype in fw_env.h

[Andreas Fenkart]

checkpatch complains about in succeding patch. Prefer to fix all
declarations in a dedicated patch.

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.h | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
index 57149e7..7345922 100644
--- a/tools/env/fw_env.h
+++ b/tools/env/fw_env.h
@@ -78,12 +78,12 @@ extern struct setenv_args setenv_args;
 
 int parse_aes_key(char *key, uint8_t *bin_key);
 
-extern int   fw_printenv(int argc, char *argv[]);
-extern char *fw_getenv  (char *name);
-extern int fw_setenv  (int argc, char *argv[]);
-extern int fw_parse_script(char *fname);
-extern int fw_env_open(void);
-extern int fw_env_write(char *name, char *value);
-extern int fw_env_close(void);
+int   fw_printenv(int argc, char *argv[]);
+char *fw_getenv(char *name);
+int fw_setenv(int argc, char *argv[]);
+int fw_parse_script(char *fname);
+int fw_env_open(void);
+int fw_env_write(char *name, char *value);
+int fw_env_close(void);
 
-extern unsignedlong  crc32  (unsigned long, const unsigned char *, 
unsigned);
+unsigned long crc32(unsigned long, const unsigned char *, unsigned);
-- 
2.8.0.rc3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 5/5] tools/env: no global variable sharing between application and library

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c  | 50 +++--
 tools/env/fw_env.h  | 25 +++--
 tools/env/fw_env_main.c | 28 +--
 3 files changed, 48 insertions(+), 55 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 3525563..9edefd7 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -35,10 +35,6 @@
 
 #include "fw_env.h"
 
-struct common_args common_args;
-struct printenv_args printenv_args;
-struct setenv_args setenv_args;
-
 #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
 
 #define min(x, y) ({   \
@@ -121,7 +117,7 @@ static unsigned char obsolete_flag = 0;
 
 static int flash_io (int mode);
 static char *envmatch (char * s1, char * s2);
-static int parse_config (void);
+static int parse_config(struct env_opts *opts);
 
 #if defined(CONFIG_FILE)
 static int get_config (char *);
@@ -229,12 +225,12 @@ int parse_aes_key(char *key, uint8_t *bin_key)
  * Print the current definition of one, or more, or all
  * environment variables
  */
-int fw_printenv(int argc, char *argv[], int value_only)
+int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts)
 {
char *env, *nxt;
int i, rc = 0;
 
-   if (fw_env_open())
+   if (fw_env_open(opts))
return -1;
 
if (argc == 0) {/* Print all env variables  */
@@ -290,12 +286,13 @@ int fw_printenv(int argc, char *argv[], int value_only)
return rc;
 }
 
-int fw_env_close(void)
+int fw_env_close(struct env_opts *opts)
 {
int ret;
-   if (common_args.aes_flag) {
+
+   if (opts->aes_flag) {
ret = env_aes_cbc_crypt(environment.data, 1,
-   common_args.aes_key);
+   opts->aes_key);
if (ret) {
fprintf(stderr,
"Error: can't encrypt env for flash\n");
@@ -448,7 +445,7 @@ int fw_env_write(char *name, char *value)
  * modified or deleted
  *
  */
-int fw_setenv(int argc, char *argv[])
+int fw_setenv(int argc, char *argv[], struct env_opts *opts)
 {
int i;
size_t len;
@@ -462,7 +459,7 @@ int fw_setenv(int argc, char *argv[])
return -1;
}
 
-   if (fw_env_open()) {
+   if (fw_env_open(opts)) {
fprintf(stderr, "Error: environment not initialized\n");
return -1;
}
@@ -498,7 +495,7 @@ int fw_setenv(int argc, char *argv[])
 
free(value);
 
-   return fw_env_close();
+   return fw_env_close(opts);
 }
 
 /*
@@ -518,7 +515,7 @@ int fw_setenv(int argc, char *argv[])
  * 0 - OK
  * -1 - Error
  */
-int fw_parse_script(char *fname)
+int fw_parse_script(char *fname, struct env_opts *opts)
 {
FILE *fp;
char dump[1024];/* Maximum line length in the file */
@@ -528,7 +525,7 @@ int fw_parse_script(char *fname)
int len;
int ret = 0;
 
-   if (fw_env_open()) {
+   if (fw_env_open(opts)) {
fprintf(stderr, "Error: environment not initialized\n");
return -1;
}
@@ -616,10 +613,9 @@ int fw_parse_script(char *fname)
if (strcmp(fname, "-") != 0)
fclose(fp);
 
-   ret |= fw_env_close();
+   ret |= fw_env_close(opts);
 
return ret;
-
 }
 
 /*
@@ -1130,7 +1126,7 @@ static char *envmatch (char * s1, char * s2)
 /*
  * Prevent confusion if running from erased flash memory
  */
-int fw_env_open(void)
+int fw_env_open(struct env_opts *opts)
 {
int crc0, crc0_ok;
unsigned char flag0;
@@ -1145,7 +1141,7 @@ int fw_env_open(void)
struct env_image_single *single;
struct env_image_redundant *redundant;
 
-   if (parse_config ())/* should fill envdevices */
+   if (parse_config(opts)) /* should fill envdevices */
return -1;
 
addr0 = calloc(1, CUR_ENVSIZE);
@@ -1177,9 +1173,9 @@ int fw_env_open(void)
 
crc0 = crc32 (0, (uint8_t *) environment.data, ENV_SIZE);
 
-   if (common_args.aes_flag) {
+   if (opts->aes_flag) {
ret = env_aes_cbc_crypt(environment.data, 0,
-   common_args.aes_key);
+   opts->aes_key);
if (ret)
return ret;
}
@@ -1235,9 +1231,9 @@ int fw_env_open(void)
 
crc1 = crc32 (0, (uint8_t *) redundant->data, ENV_SIZE);
 
-   if (common_args.aes_flag) {
+   if (opts->aes_flag) {
ret = env_aes_cbc_crypt(redundant->data, 0,
-   common_args.aes_key);
+ 

[U-Boot] [PATCH 4/5] tools/env: compute size of usable area only once

[Andreas Fenkart]

for double buffering to work, redundant buffers must have equal size

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 32 ++--
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index bd218cb..3525563 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -75,7 +75,8 @@ static int dev_current;
 
 #define CUR_ENVSIZE ENVSIZE(dev_current)
 
-#define ENV_SIZE  getenvsize()
+static unsigned long usable_envsize;
+#define ENV_SIZE  usable_envsize
 
 struct env_image_single {
uint32_tcrc;/* CRC32 over data bytes*/
@@ -125,18 +126,6 @@ static int parse_config (void);
 #if defined(CONFIG_FILE)
 static int get_config (char *);
 #endif
-static inline ulong getenvsize (void)
-{
-   ulong rc = CUR_ENVSIZE - sizeof(uint32_t);
-
-   if (HaveRedundEnv)
-   rc -= sizeof (char);
-
-   if (common_args.aes_flag)
-   rc &= ~(AES_KEY_LENGTH - 1);
-
-   return rc;
-}
 
 static char *skip_chars(char *s)
 {
@@ -955,7 +944,7 @@ static int env_aes_cbc_crypt(char *payload, const int enc,
 uint8_t key[AES_KEY_LENGTH])
 {
uint8_t *data = (uint8_t *)payload;
-   const int len = getenvsize();
+   const int len = usable_envsize;
uint8_t key_exp[AES_EXPAND_KEY_LENGTH];
uint32_t aes_blocks;
 
@@ -1381,6 +1370,21 @@ static int parse_config ()
DEVNAME (1), strerror (errno));
return -1;
}
+
+   if (HaveRedundEnv && ENVSIZE(0) != ENVSIZE(1)) {
+   ENVSIZE(0) = ENVSIZE(1) = min(ENVSIZE(0), ENVSIZE(1));
+   fprintf(stderr,
+   "Redundant environments have inequal size, set to 
0x%08lx\n",
+   ENVSIZE(1));
+   }
+
+   usable_envsize = CUR_ENVSIZE - sizeof(uint32_t);
+   if (HaveRedundEnv)
+   usable_envsize -= sizeof(char);
+
+   if (common_args.aes_flag)
+   usable_envsize &= ~(AES_KEY_LENGTH - 1);
+
return 0;
 }
 
-- 
2.8.0.rc3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 3/5] tools/env: fw_printenv pass value_only as argument

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c  | 6 +++---
 tools/env/fw_env.h  | 4 ++--
 tools/env/fw_env_main.c | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index c362a41..bd218cb 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -240,7 +240,7 @@ int parse_aes_key(char *key, uint8_t *bin_key)
  * Print the current definition of one, or more, or all
  * environment variables
  */
-int fw_printenv (int argc, char *argv[])
+int fw_printenv(int argc, char *argv[], int value_only)
 {
char *env, *nxt;
int i, rc = 0;
@@ -263,7 +263,7 @@ int fw_printenv (int argc, char *argv[])
return 0;
}
 
-   if (printenv_args.name_suppress && argc != 1) {
+   if (value_only && argc != 1) {
fprintf(stderr,
"## Error: `-n' option requires exactly one 
argument\n");
return -1;
@@ -284,7 +284,7 @@ int fw_printenv (int argc, char *argv[])
}
val = envmatch (name, env);
if (val) {
-   if (!printenv_args.name_suppress) {
+   if (!value_only) {
fputs (name, stdout);
putc ('=', stdout);
}
diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
index 7345922..d4daeea 100644
--- a/tools/env/fw_env.h
+++ b/tools/env/fw_env.h
@@ -67,7 +67,7 @@ struct common_args {
 extern struct common_args common_args;
 
 struct printenv_args {
-   int name_suppress;
+   int value_only;
 };
 extern struct printenv_args printenv_args;
 
@@ -78,7 +78,7 @@ extern struct setenv_args setenv_args;
 
 int parse_aes_key(char *key, uint8_t *bin_key);
 
-int   fw_printenv(int argc, char *argv[]);
+int fw_printenv(int argc, char *argv[], int value_only);
 char *fw_getenv(char *name);
 int fw_setenv(int argc, char *argv[]);
 int fw_parse_script(char *fname);
diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index 3706d8f..2a45a0d 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -151,7 +151,7 @@ int parse_printenv_args(int argc, char *argv[])
   EOF) {
switch (c) {
case 'n':
-   printenv_args.name_suppress = 1;
+   printenv_args.value_only = 1;
break;
case 'a':
case 'c':
@@ -240,7 +240,7 @@ int main(int argc, char *argv[])
}
 
if (do_printenv) {
-   if (fw_printenv(argc, argv) != 0)
+   if (fw_printenv(argc, argv, printenv_args.value_only))
retval = EXIT_FAILURE;
} else {
if (!setenv_args.script_file) {
-- 
2.8.0.rc3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/5] tools/env: make env_aes_cbc_crypt re-entrant

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 1420ac5..c362a41 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -106,7 +106,8 @@ static struct environment environment = {
.flag_scheme = FLAG_NONE,
 };
 
-static int env_aes_cbc_crypt(char *data, const int enc);
+static int env_aes_cbc_crypt(char *data, const int enc,
+uint8_t key[AES_KEY_LENGTH]);
 
 static int HaveRedundEnv = 0;
 
@@ -304,7 +305,8 @@ int fw_env_close(void)
 {
int ret;
if (common_args.aes_flag) {
-   ret = env_aes_cbc_crypt(environment.data, 1);
+   ret = env_aes_cbc_crypt(environment.data, 1,
+   common_args.aes_key);
if (ret) {
fprintf(stderr,
"Error: can't encrypt env for flash\n");
@@ -949,7 +951,8 @@ static int flash_flag_obsolete (int dev, int fd, off_t 
offset)
 }
 
 /* Encrypt or decrypt the environment before writing or reading it. */
-static int env_aes_cbc_crypt(char *payload, const int enc)
+static int env_aes_cbc_crypt(char *payload, const int enc,
+uint8_t key[AES_KEY_LENGTH])
 {
uint8_t *data = (uint8_t *)payload;
const int len = getenvsize();
@@ -957,7 +960,7 @@ static int env_aes_cbc_crypt(char *payload, const int enc)
uint32_t aes_blocks;
 
/* First we expand the key. */
-   aes_expand_key(common_args.aes_key, key_exp);
+   aes_expand_key(key, key_exp);
 
/* Calculate the number of AES blocks to encrypt. */
aes_blocks = DIV_ROUND_UP(len, AES_KEY_LENGTH);
@@ -1186,7 +1189,8 @@ int fw_env_open(void)
crc0 = crc32 (0, (uint8_t *) environment.data, ENV_SIZE);
 
if (common_args.aes_flag) {
-   ret = env_aes_cbc_crypt(environment.data, 0);
+   ret = env_aes_cbc_crypt(environment.data, 0,
+   common_args.aes_key);
if (ret)
return ret;
}
@@ -1243,7 +1247,8 @@ int fw_env_open(void)
crc1 = crc32 (0, (uint8_t *) redundant->data, ENV_SIZE);
 
if (common_args.aes_flag) {
-   ret = env_aes_cbc_crypt(redundant->data, 0);
+   ret = env_aes_cbc_crypt(redundant->data, 0,
+   common_args.aes_key);
if (ret)
return ret;
}
-- 
2.8.0.rc3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 0/5] tools/env: pass arguments as parameters not global structure

[Andreas Fenkart]

u-boot tools can be built as a library (ibubootenv.a). Passing arguments
to the library using global structures is not a good interface.

Andreas Fenkart (5):
  tools/env: make env_aes_cbc_crypt re-entrant
  tools/env: remove 'extern' from function prototype in fw_env.h
  tools/env: fw_printenv pass value_only as argument
  tools/env: compute size of usable area only once
  tools/env: no global variable sharing between application and library

 tools/env/fw_env.c  | 95 ++---
 tools/env/fw_env.h  | 31 ++--
 tools/env/fw_env_main.c | 28 +--
 3 files changed, 78 insertions(+), 76 deletions(-)

-- 
2.8.0.rc3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] ENV library broken with setenv/printenv argument structs

[Andreas Fenkart]

HI Stefano,

I was not aware of the use case. I have sent out a simple patch that
moves the definition of the structs from fw_main_env.c to fw_env.c,
which should avoid the linking problem. Do you have test application
you could send me. I without doing the option parsing as done in
fw_env_main, you probably use only use a very limited subset of the
library.

/Andi

2016-03-25 10:31 GMT+01:00 Stefano Babic :
> Hi Andreas,
>
> your series for the U-Boot environment tools does not let to use anymore
> the tools as library to be linked to custom program.
>
> In fact, tools/env/Makefile generates a library too:
>
> lib-y += fw_env.o \
> crc32.o ctype.o linux_string.o \
> env_attr.o env_flags.o aes.o
>
> that means that fw_main should not maintain any structure that is
> referenced by the library.
> In fact, linking with the library (renamed as ibubootenv.a), gives the
> errors:
>
> undefined reference to `printenv_args'
> undefined reference to `common_args'
>
> The two functions fw_printenv() and fw_setenv() are then supposed to be
> called by a custom program, what it does not work anymore.
>
> Can you take a look at it ?
>
> Thanks,
> Stefano Babic
>
> --
> =
> DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
> =
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] tools: env: bug: config structs must be defined in tools library

[Andreas Fenkart]

fw_senten/fw_printenv can be compiled as a tools library,
excluding the fw_env_main object.

Reported-by: Stefano Babic <sba...@denx.de>
Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c  | 4 
 tools/env/fw_env_main.c | 4 
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index ee17a69..2533dc4 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -34,6 +34,10 @@
 
 #include "fw_env.h"
 
+struct common_args common_args;
+struct printenv_args printenv_args;
+struct setenv_args setenv_args;
+
 #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
 
 #define WHITESPACE(c) ((c == '\t') || (c == ' '))
diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index 4bd4216..3065de9 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -49,10 +49,6 @@ static struct option long_options[] = {
{NULL, 0, NULL, 0}
 };
 
-struct common_args common_args;
-struct printenv_args printenv_args;
-struct setenv_args setenv_args;
-
 void usage_printenv(void)
 {
 
-- 
2.8.0.rc3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 4/6] tools: env: fw_parse_script: simplify removal of newline/carriage return

[Andreas Fenkart]

fgets returns when the first '\n' is found

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 60574f2..5c7505c 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -566,14 +566,12 @@ int fw_parse_script(char *fname)
}
 
/* Drop ending line feed / carriage return */
-   while (len > 0 && (dump[len - 1] == '\n' ||
-   dump[len - 1] == '\r')) {
-   dump[len - 1] = '\0';
-   len--;
-   }
+   dump[--len] = '\0';
+   if (len && dump[len - 1] == '\r')
+   dump[--len] = '\0';
 
/* Skip comment or empty lines */
-   if ((len == 0) || dump[0] == '#')
+   if (len == 0 || dump[0] == '#')
continue;
 
/*
-- 
2.7.0

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 3/6] tools: env: split fw_string_blank into skip_chars / skip_blanks

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 23 ++-
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 0a438a3..60574f2 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -133,14 +133,19 @@ static inline ulong getenvsize (void)
return rc;
 }
 
-static char *fw_string_blank(char *s, int noblank)
+static char *skip_chars(char *s)
 {
-   int i;
-   int len = strlen(s);
+   for (; *s != '\0'; s++) {
+   if (isblank(*s))
+   return s;
+   }
+   return NULL;
+}
 
-   for (i = 0; i < len; i++, s++) {
-   if ((noblank && !isblank(*s)) ||
-   (!noblank && isblank(*s)))
+static char *skip_blanks(char *s)
+{
+   for (; *s != '\0'; s++) {
+   if (!isblank(*s))
return s;
}
return NULL;
@@ -575,17 +580,17 @@ int fw_parse_script(char *fname)
 * Search for variable's name,
 * remove leading whitespaces
 */
-   name = fw_string_blank(dump, 1);
+   name = skip_blanks(dump);
if (!name)
continue;
 
/* The first white space is the end of variable name */
-   val = fw_string_blank(name, 0);
+   val = skip_chars(name);
len = strlen(name);
if (val) {
*val++ = '\0';
if ((val - name) < len)
-   val = fw_string_blank(val, 1);
+   val = skip_blanks(val);
else
val = NULL;
}
-- 
2.7.0

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/6] tools: env: fw_string_blank: return from loop when item found

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index bded9f6..0a438a3 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -141,12 +141,9 @@ static char *fw_string_blank(char *s, int noblank)
for (i = 0; i < len; i++, s++) {
if ((noblank && !isblank(*s)) ||
(!noblank && isblank(*s)))
-   break;
+   return s;
}
-   if (i == len)
-   return NULL;
-
-   return s;
+   return NULL;
 }
 
 /*
-- 
2.7.0

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/6] tools: env: replace WHITESPACE macro by isblank

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index ee17a69..bded9f6 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -36,8 +37,6 @@
 
 #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
 
-#define WHITESPACE(c) ((c == '\t') || (c == ' '))
-
 #define min(x, y) ({   \
typeof(x) _min1 = (x);  \
typeof(y) _min2 = (y);  \
@@ -140,8 +139,8 @@ static char *fw_string_blank(char *s, int noblank)
int len = strlen(s);
 
for (i = 0; i < len; i++, s++) {
-   if ((noblank && !WHITESPACE(*s)) ||
-   (!noblank && WHITESPACE(*s)))
+   if ((noblank && !isblank(*s)) ||
+   (!noblank && isblank(*s)))
break;
}
if (i == len)
-- 
2.7.0

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 0/6] tools: env: trivial cleanups in script parsing

[Andreas Fenkart]

Andreas Fenkart (6):
  tools: env: replace WHITESPACE macro by isblank
  tools: env: fw_string_blank: return from loop when item found
  tools: env: split fw_string_blank into skip_chars / skip_blanks
  tools: env: fw_parse_script: simplify removal of newline/carriage
return
  tools: env: fw_parse_script: simplify value parsing
  tools: env: fw_parse_script: align arguments with opening brace

 tools/env/fw_env.c | 49 ++---
 1 file changed, 22 insertions(+), 27 deletions(-)

-- 
2.7.0

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 7/7] tools: env: update usage strings

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env_main.c | 117 +++-
 1 file changed, 75 insertions(+), 42 deletions(-)

diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index 39c8771..4bd4216 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -36,12 +36,16 @@
 #include 
 #include "fw_env.h"
 
-#defineCMD_PRINTENV"fw_printenv"
+#define CMD_PRINTENV   "fw_printenv"
 #define CMD_SETENV "fw_setenv"
+static int do_printenv;
 
 static struct option long_options[] = {
-   {"script", required_argument, NULL, 's'},
+   {"aes", required_argument, NULL, 'a'},
+   {"config", required_argument, NULL, 'c'},
{"help", no_argument, NULL, 'h'},
+   {"script", required_argument, NULL, 's'},
+   {"noheader", required_argument, NULL, 'n'},
{NULL, 0, NULL, 0}
 };
 
@@ -49,36 +53,58 @@ struct common_args common_args;
 struct printenv_args printenv_args;
 struct setenv_args setenv_args;
 
-void usage(void)
+void usage_printenv(void)
 {
 
-   fprintf(stderr, "fw_printenv/fw_setenv, "
-   "a command line interface to U-Boot environment\n\n"
-#ifndef CONFIG_FILE
-   "usage:\tfw_printenv [-a key] [-n] [variable name]\n"
-   "\tfw_setenv [-a key] [variable name] [variable value]\n"
-#else
-   "usage:\tfw_printenv [-c /my/fw_env.config] [-a key] [-n] 
[variable name]\n"
-   "\tfw_setenv [-c /my/fw_env.config] [-a key] [variable name] 
[variable value]\n"
+   fprintf(stderr,
+   "Usage: fw_printenv [OPTIONS]... [VARIABLE]...\n"
+   "Print variables from U-Boot environment\n"
+   "\n"
+   " -h, --help   print this help.\n"
+#ifdef CONFIG_ENV_AES
+   " -a, --aesaes key to access environment\n"
 #endif
-   "\tfw_setenv -s [ file ]\n"
-   "\tfw_setenv -s - < [ file ]\n\n"
-   "The file passed as argument contains only pairs "
-   "name / value\n"
-   "Example:\n"
-   "# Any line starting with # is treated as comment\n"
+#ifdef CONFIG_FILE
+   " -c, --config configuration file, default:" 
CONFIG_FILE "\n"
+#endif
+   " -n, --noheader   do not repeat variable name in output\n"
+   "\n");
+}
+
+void usage_setenv(void)
+{
+   fprintf(stderr,
+   "Usage: fw_setenv [OPTIONS]... [VARIABLE]...\n"
+   "Modify variables in U-Boot environment\n"
+   "\n"
+   " -h, --help   print this help.\n"
+#ifdef CONFIG_ENV_AES
+   " -a, --aesaes key to access environment\n"
+#endif
+#ifdef CONFIG_FILE
+   " -c, --config configuration file, default:" 
CONFIG_FILE "\n"
+#endif
+   " -s, --script batch mode to minimize writes\n"
+   "\n"
+   "Examples:\n"
+   "  fw_setenv foo bar   set variable foo equal bar\n"
+   "  fw_setenv foo   clear variable foo\n"
+   "  fw_setenv --script file run batch script\n"
"\n"
-   "\t  netdev eth0\n"
-   "\t  kernel_addr40\n"
-   "\t  var1\n"
-   "\t  var2  The quick brown fox jumps over the "
-   "lazy dog\n"
+   "Script Syntax:\n"
+   "  key [space] value\n"
+   "  lines starting with '#' are treated as commment\n"
"\n"
-   "A variable without value will be dropped. It is possible\n"
-   "to put any number of spaces between the fields, but any\n"
-   "space inside the value is treated as part of the value "
-   "itself.\n\n"
-   );
+   "  A variable without value will be deleted. Any number of 
spaces are\n"
+   "  allowed between key and value. Space inside of the value is 
treated\n"
+   "  as part of the value itself.\n"
+   "\n"
+   "Script Example:\n"
+   "  netdev eth0\n"
+   "  kernel_addr40\n"
+   "  fooempty 

[U-Boot] [PATCH v3 6/7] tools: env: factor out parse_common_args

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env_main.c | 62 +
 1 file changed, 37 insertions(+), 25 deletions(-)

diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index daf4688..39c8771 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -81,7 +81,7 @@ void usage(void)
);
 }
 
-int parse_printenv_args(int argc, char *argv[])
+static void parse_common_args(int argc, char *argv[])
 {
int c;
 
@@ -89,13 +89,13 @@ int parse_printenv_args(int argc, char *argv[])
common_args.config_file = CONFIG_FILE;
 #endif
 
-   while ((c = getopt_long (argc, argv, "a:c:ns:h",
-   long_options, NULL)) != EOF) {
+   while ((c = getopt_long(argc, argv, ":a:c:h", long_options, NULL)) !=
+  EOF) {
switch (c) {
case 'a':
if (parse_aes_key(optarg, common_args.aes_key)) {
fprintf(stderr, "AES key parse error\n");
-   return EXIT_FAILURE;
+   exit(EXIT_FAILURE);
}
common_args.aes_flag = 1;
break;
@@ -104,12 +104,37 @@ int parse_printenv_args(int argc, char *argv[])
common_args.config_file = optarg;
break;
 #endif
+   case 'h':
+   usage();
+   exit(EXIT_SUCCESS);
+   break;
+   default:
+   /* ignore unknown options */
+   break;
+   }
+   }
+
+   /* Reset getopt for the next pass. */
+   opterr = 1;
+   optind = 1;
+}
+
+int parse_printenv_args(int argc, char *argv[])
+{
+   int c;
+
+   parse_common_args(argc, argv);
+
+   while ((c = getopt_long(argc, argv, "a:c:ns:h", long_options, NULL)) !=
+  EOF) {
+   switch (c) {
case 'n':
printenv_args.name_suppress = 1;
break;
+   case 'a':
+   case 'c':
case 'h':
-   usage();
-   exit(EXIT_SUCCESS);
+   /* ignore common options */
break;
default: /* '?' */
usage();
@@ -124,31 +149,18 @@ int parse_setenv_args(int argc, char *argv[])
 {
int c;
 
-#ifdef CONFIG_FILE
-   common_args.config_file = CONFIG_FILE;
-#endif
+   parse_common_args(argc, argv);
 
-   while ((c = getopt_long (argc, argv, "a:c:ns:h",
-   long_options, NULL)) != EOF) {
+   while ((c = getopt_long(argc, argv, "a:c:ns:h", long_options, NULL)) !=
+  EOF) {
switch (c) {
-   case 'a':
-   if (parse_aes_key(optarg, common_args.aes_key)) {
-   fprintf(stderr, "AES key parse error\n");
-   return EXIT_FAILURE;
-   }
-   common_args.aes_flag = 1;
-   break;
-#ifdef CONFIG_FILE
-   case 'c':
-   common_args.config_file = optarg;
-   break;
-#endif
case 's':
setenv_args.script_file = optarg;
break;
+   case 'a':
+   case 'c':
case 'h':
-   usage();
-   exit(EXIT_SUCCESS);
+   /* ignore common options */
break;
default: /* '?' */
usage();
-- 
2.6.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 1/7] tools: env validate: pass values as 0-based array

[Andreas Fenkart]

passing argv/argc can produce off-by-one errors

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 common/env_flags.c  | 14 +++---
 include/env_flags.h |  2 +-
 tools/env/fw_env.c  | 11 +++
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/common/env_flags.c b/common/env_flags.c
index e682d85..529e371 100644
--- a/common/env_flags.c
+++ b/common/env_flags.c
@@ -373,21 +373,21 @@ int env_flags_validate_varaccess(const char *name, int 
check_mask)
 /*
  * Validate the parameters to "env set" directly
  */
-int env_flags_validate_env_set_params(int argc, char * const argv[])
+int env_flags_validate_env_set_params(char *name, char * const val[], int 
count)
 {
-   if ((argc >= 3) && argv[2] != NULL) {
-   enum env_flags_vartype type = env_flags_get_type(argv[1]);
+   if ((count >= 1) && val[0] != NULL) {
+   enum env_flags_vartype type = env_flags_get_type(name);
 
/*
 * we don't currently check types that need more than
 * one argument
 */
-   if (type != env_flags_vartype_string && argc > 3) {
-   printf("## Error: too many parameters for setting "
-   "\"%s\"\n", argv[1]);
+   if (type != env_flags_vartype_string && count > 1) {
+   printf("## Error: too many parameters for setting 
\"%s\"\n",
+  name);
return -1;
}
-   return env_flags_validate_type(argv[1], argv[2]);
+   return env_flags_validate_type(name, val[0]);
}
/* ok */
return 0;
diff --git a/include/env_flags.h b/include/env_flags.h
index 8823fb9..9e87e1b 100644
--- a/include/env_flags.h
+++ b/include/env_flags.h
@@ -143,7 +143,7 @@ int env_flags_validate_varaccess(const char *name, int 
check_mask);
 /*
  * Validate the parameters passed to "env set" for type compliance
  */
-int env_flags_validate_env_set_params(int argc, char * const argv[]);
+int env_flags_validate_env_set_params(char *name, char *const val[], int 
count);
 
 #else /* !USE_HOSTCC */
 
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index ba11f77..22507f6 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -497,8 +497,9 @@ int fw_setenv(int argc, char *argv[])
 {
int i, rc;
size_t len;
-   char *name;
+   char *name, **valv;
char *value = NULL;
+   int valc;
 
 #ifdef CONFIG_FILE
if (argc >= 2 && strcmp(argv[1], "-c") == 0) {
@@ -542,13 +543,15 @@ int fw_setenv(int argc, char *argv[])
}
 
name = argv[1];
+   valv = argv + 2;
+   valc = argc - 2;
 
-   if (env_flags_validate_env_set_params(argc, argv) < 0)
+   if (env_flags_validate_env_set_params(name, valv, valc) < 0)
return 1;
 
len = 0;
-   for (i = 2; i < argc; ++i) {
-   char *val = argv[i];
+   for (i = 0; i < valc; ++i) {
+   char *val = valv[i];
size_t val_len = strlen(val);
 
if (value)
-- 
2.6.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 0/7] tools: env: simplify argument parsing

[Andreas Fenkart]

Currently parameter parsing happens in two steps. First with 
getopt at the beginning of main to parse the common options.
Then second is using adhoc parsing in printenv/setenv code path.
When doing adhoc parsing, the order of arguments is important.
This patch will parse arguments only in one place using getopt
and store the parsed flags in a global struct.

v3:
- rebased on top of master/8555dd8
- made emails in From:/Signed-off header match

Andreas Fenkart (7):
  tools: env validate: pass values as 0-based array
  tools: env: make parse_aes_key stateless
  tools: env: introduce setenv/printenv argument structs
  tools: env: parse aes key / suppress flag into argument struct
  tools: env: shift optind arguments and fix argument indices
  tools: env: factor out parse_common_args
  tools: env: update usage strings

 common/env_flags.c  |  14 +--
 include/env_flags.h |   2 +-
 tools/env/fw_env.c  | 127 ++--
 tools/env/fw_env.h  |  24 +
 tools/env/fw_env_main.c | 249 ++--
 5 files changed, 239 insertions(+), 177 deletions(-)

-- 
2.6.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 2/7] tools: env: make parse_aes_key stateless

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 22507f6..5b76b74 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -207,7 +207,7 @@ char *fw_getdefenv(char *name)
return NULL;
 }
 
-static int parse_aes_key(char *key)
+static int parse_aes_key(char *key, uint8_t *bin_key)
 {
char tmp[5] = { '0', 'x', 0, 0, 0 };
unsigned long ul;
@@ -229,11 +229,9 @@ static int parse_aes_key(char *key)
"## Error: '-a' option requires valid AES 
key\n");
return -1;
}
-   aes_key[i] = ul & 0xff;
+   bin_key[i] = ul & 0xff;
key += 2;
}
-   aes_flag = 1;
-
return 0;
 }
 
@@ -266,9 +264,10 @@ int fw_printenv (int argc, char *argv[])
"## Error: '-a' option requires AES key\n");
return -1;
}
-   rc = parse_aes_key(argv[2]);
+   rc = parse_aes_key(argv[2], aes_key);
if (rc)
return rc;
+   aes_flag = 1;
argv += 2;
argc -= 2;
}
@@ -525,9 +524,10 @@ int fw_setenv(int argc, char *argv[])
"## Error: '-a' option requires AES key\n");
return -1;
}
-   rc = parse_aes_key(argv[2]);
+   rc = parse_aes_key(argv[2], aes_key);
if (rc)
return rc;
+   aes_flag = 1;
argv += 2;
argc -= 2;
}
-- 
2.6.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 4/7] tools: env: parse aes key / suppress flag into argument struct

[Andreas Fenkart]

disabled original parsing, but not yet removed since the
argument indexing needs to be fixed

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c  | 64 ++---
 tools/env/fw_env.h  | 15 
 tools/env/fw_env_main.c | 31 
 3 files changed, 53 insertions(+), 57 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 5b76b74..2cfff02 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -33,8 +33,6 @@
 
 #include "fw_env.h"
 
-#include 
-
 #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
 
 #define WHITESPACE(c) ((c == '\t') || (c == ' '))
@@ -104,9 +102,6 @@ static struct environment environment = {
.flag_scheme = FLAG_NONE,
 };
 
-/* Is AES encryption used? */
-static int aes_flag;
-static uint8_t aes_key[AES_KEY_LENGTH] = { 0 };
 static int env_aes_cbc_crypt(char *data, const int enc);
 
 static int HaveRedundEnv = 0;
@@ -124,7 +119,6 @@ static int parse_config (void);
 
 #if defined(CONFIG_FILE)
 static int get_config (char *);
-static char *config_file = CONFIG_FILE;
 #endif
 static inline ulong getenvsize (void)
 {
@@ -133,7 +127,7 @@ static inline ulong getenvsize (void)
if (HaveRedundEnv)
rc -= sizeof (char);
 
-   if (aes_flag)
+   if (common_args.aes_flag)
rc &= ~(AES_KEY_LENGTH - 1);
 
return rc;
@@ -207,7 +201,7 @@ char *fw_getdefenv(char *name)
return NULL;
 }
 
-static int parse_aes_key(char *key, uint8_t *bin_key)
+int parse_aes_key(char *key, uint8_t *bin_key)
 {
char tmp[5] = { '0', 'x', 0, 0, 0 };
unsigned long ul;
@@ -242,32 +236,16 @@ static int parse_aes_key(char *key, uint8_t *bin_key)
 int fw_printenv (int argc, char *argv[])
 {
char *env, *nxt;
-   int i, n_flag;
-   int rc = 0;
+   int i, rc = 0;
 
 #ifdef CONFIG_FILE
if (argc >= 2 && strcmp(argv[1], "-c") == 0) {
-   if (argc < 3) {
-   fprintf(stderr,
-   "## Error: '-c' option requires the config file 
to use\n");
-   return -1;
-   }
-   config_file = argv[2];
argv += 2;
argc -= 2;
}
 #endif
 
if (argc >= 2 && strcmp(argv[1], "-a") == 0) {
-   if (argc < 3) {
-   fprintf(stderr,
-   "## Error: '-a' option requires AES key\n");
-   return -1;
-   }
-   rc = parse_aes_key(argv[2], aes_key);
-   if (rc)
-   return rc;
-   aes_flag = 1;
argv += 2;
argc -= 2;
}
@@ -291,7 +269,6 @@ int fw_printenv (int argc, char *argv[])
}
 
if (strcmp (argv[1], "-n") == 0) {
-   n_flag = 1;
++argv;
--argc;
if (argc != 2) {
@@ -299,8 +276,6 @@ int fw_printenv (int argc, char *argv[])
"`-n' option requires exactly one argument\n");
return -1;
}
-   } else {
-   n_flag = 0;
}
 
for (i = 1; i < argc; ++i) {/* print single env variables   */
@@ -318,7 +293,7 @@ int fw_printenv (int argc, char *argv[])
}
val = envmatch (name, env);
if (val) {
-   if (!n_flag) {
+   if (!printenv_args.name_suppress) {
fputs (name, stdout);
putc ('=', stdout);
}
@@ -338,7 +313,7 @@ int fw_printenv (int argc, char *argv[])
 int fw_env_close(void)
 {
int ret;
-   if (aes_flag) {
+   if (common_args.aes_flag) {
ret = env_aes_cbc_crypt(environment.data, 1);
if (ret) {
fprintf(stderr,
@@ -494,7 +469,7 @@ int fw_env_write(char *name, char *value)
  */
 int fw_setenv(int argc, char *argv[])
 {
-   int i, rc;
+   int i;
size_t len;
char *name, **valv;
char *value = NULL;
@@ -502,12 +477,6 @@ int fw_setenv(int argc, char *argv[])
 
 #ifdef CONFIG_FILE
if (argc >= 2 && strcmp(argv[1], "-c") == 0) {
-   if (argc < 3) {
-   fprintf(stderr,
-   "## Error: '-c' option requires the config file 
to use\n");
-   return -1;
-   }
-   config_file = argv[2];
argv += 2;
argc -= 2;
}
@@ -519,15 +488,6 @@ int fw_setenv(int argc, char *argv[])
}
 
if (strcmp(argv[1], "-a&quo

[U-Boot] [PATCH v3 3/7] tools: env: introduce setenv/printenv argument structs

[Andreas Fenkart]

goal is to use getopt for all argument parsing instead of adhoc
parsing in fw_getenv/fw_setenv functions

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.h  |   9 
 tools/env/fw_env_main.c | 113 
 2 files changed, 84 insertions(+), 38 deletions(-)

diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
index 60c0517..1a02c46 100644
--- a/tools/env/fw_env.h
+++ b/tools/env/fw_env.h
@@ -54,6 +54,15 @@
"bootm"
 #endif
 
+struct printenv_args {
+};
+extern struct printenv_args printenv_args;
+
+struct setenv_args {
+   char *script_file;
+};
+extern struct setenv_args setenv_args;
+
 extern int   fw_printenv(int argc, char *argv[]);
 extern char *fw_getenv  (char *name);
 extern int fw_setenv  (int argc, char *argv[]);
diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index 234c061..0c9f918 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -45,6 +45,9 @@ static struct option long_options[] = {
{NULL, 0, NULL, 0}
 };
 
+struct printenv_args printenv_args;
+struct setenv_args setenv_args;
+
 void usage(void)
 {
 
@@ -77,31 +80,9 @@ void usage(void)
);
 }
 
-int main(int argc, char *argv[])
+int parse_printenv_args(int argc, char *argv[])
 {
-   char *p;
-   char *cmdname = *argv;
-   char *script_file = NULL;
int c;
-   const char *lockname = "/var/lock/" CMD_PRINTENV ".lock";
-   int lockfd = -1;
-   int retval = EXIT_SUCCESS;
-
-   lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC, 0666);
-   if (-1 == lockfd) {
-   fprintf(stderr, "Error opening lock file %s\n", lockname);
-   return EXIT_FAILURE;
-   }
-
-   if (-1 == flock(lockfd, LOCK_EX)) {
-   fprintf(stderr, "Error locking file %s\n", lockname);
-   close(lockfd);
-   return EXIT_FAILURE;
-   }
-
-   if ((p = strrchr (cmdname, '/')) != NULL) {
-   cmdname = p + 1;
-   }
 
while ((c = getopt_long (argc, argv, "a:c:ns:h",
long_options, NULL)) != EOF) {
@@ -115,40 +96,96 @@ int main(int argc, char *argv[])
case 'n':
/* handled in fw_printenv */
break;
+   case 'h':
+   usage();
+   exit(EXIT_SUCCESS);
+   break;
+   default: /* '?' */
+   usage();
+   exit(EXIT_FAILURE);
+   break;
+   }
+   }
+   return 0;
+}
+
+int parse_setenv_args(int argc, char *argv[])
+{
+   int c;
+
+   while ((c = getopt_long (argc, argv, "a:c:ns:h",
+   long_options, NULL)) != EOF) {
+   switch (c) {
+   case 'a':
+   /* AES key, handled later */
+   break;
+   case 'c':
+   /* handled later */
+   break;
case 's':
-   script_file = optarg;
+   setenv_args.script_file = optarg;
break;
case 'h':
usage();
-   goto exit;
+   exit(EXIT_SUCCESS);
+   break;
default: /* '?' */
-   fprintf(stderr, "Try `%s --help' for more information."
-   "\n", cmdname);
-   retval = EXIT_FAILURE;
-   goto exit;
+   usage();
+   exit(EXIT_FAILURE);
+   break;
}
}
+   return 0;
+}
+
+int main(int argc, char *argv[])
+{
+   char *cmdname = *argv;
+   const char *lockname = "/var/lock/" CMD_PRINTENV ".lock";
+   int lockfd = -1;
+   int retval = EXIT_SUCCESS;
+
+   if (strrchr(cmdname, '/') != NULL)
+   cmdname = strrchr(cmdname, '/') + 1;
+
+   if (strcmp(cmdname, CMD_PRINTENV) == 0) {
+   if (parse_printenv_args(argc, argv))
+   exit(EXIT_FAILURE);
+   } else if (strcmp(cmdname, CMD_SETENV) == 0) {
+   if (parse_setenv_args(argc, argv))
+   exit(EXIT_FAILURE);
+   } else {
+   fprintf(stderr,
+   "Identity crisis - may be called as `%s' or as `%s' but 
not as `%s'\n",
+   CMD_PRINTENV, CMD_SETENV, cmdname);
+   exit(EXIT_FAILURE);
+   }
+
+   lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC, 0666);
+   if (-1 == lockfd) {
+   fprintf(stderr, "Error opening lock file %s\n", lockname);
+   return EXIT_FAILURE;
+   }
+
+ 

[U-Boot] [PATCH v3 5/7] tools: env: shift optind arguments and fix argument indices

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@digitalstrom.com>
---
 tools/env/fw_env.c  | 54 ++---
 tools/env/fw_env_main.c |  4 
 2 files changed, 15 insertions(+), 43 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 2cfff02..b1cf9ae 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -238,22 +238,10 @@ int fw_printenv (int argc, char *argv[])
char *env, *nxt;
int i, rc = 0;
 
-#ifdef CONFIG_FILE
-   if (argc >= 2 && strcmp(argv[1], "-c") == 0) {
-   argv += 2;
-   argc -= 2;
-   }
-#endif
-
-   if (argc >= 2 && strcmp(argv[1], "-a") == 0) {
-   argv += 2;
-   argc -= 2;
-   }
-
if (fw_env_open())
return -1;
 
-   if (argc == 1) {/* Print all env variables  */
+   if (argc == 0) {/* Print all env variables  */
for (env = environment.data; *env; env = nxt + 1) {
for (nxt = env; *nxt; ++nxt) {
if (nxt >= [ENV_SIZE]) {
@@ -268,17 +256,13 @@ int fw_printenv (int argc, char *argv[])
return 0;
}
 
-   if (strcmp (argv[1], "-n") == 0) {
-   ++argv;
-   --argc;
-   if (argc != 2) {
-   fprintf (stderr, "## Error: "
-   "`-n' option requires exactly one argument\n");
-   return -1;
-   }
+   if (printenv_args.name_suppress && argc != 1) {
+   fprintf(stderr,
+   "## Error: `-n' option requires exactly one 
argument\n");
+   return -1;
}
 
-   for (i = 1; i < argc; ++i) {/* print single env variables   */
+   for (i = 0; i < argc; ++i) {/* print single env variables   */
char *name = argv[i];
char *val = NULL;
 
@@ -475,24 +459,8 @@ int fw_setenv(int argc, char *argv[])
char *value = NULL;
int valc;
 
-#ifdef CONFIG_FILE
-   if (argc >= 2 && strcmp(argv[1], "-c") == 0) {
-   argv += 2;
-   argc -= 2;
-   }
-#endif
-
-   if (argc < 2) {
-   errno = EINVAL;
-   return -1;
-   }
-
-   if (strcmp(argv[1], "-a") == 0) {
-   argv += 2;
-   argc -= 2;
-   }
-
-   if (argc < 2) {
+   if (argc < 1) {
+   fprintf(stderr, "## Error: variable name missing\n");
errno = EINVAL;
return -1;
}
@@ -502,9 +470,9 @@ int fw_setenv(int argc, char *argv[])
return -1;
}
 
-   name = argv[1];
-   valv = argv + 2;
-   valc = argc - 2;
+   name = argv[0];
+   valv = argv + 1;
+   valc = argc - 1;
 
if (env_flags_validate_env_set_params(name, valv, valc) < 0)
return 1;
diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index b68f1bf..daf4688 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -182,6 +182,10 @@ int main(int argc, char *argv[])
exit(EXIT_FAILURE);
}
 
+   /* shift parsed flags, jump to non-option arguments */
+   argc -= optind;
+   argv += optind;
+
lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC, 0666);
if (-1 == lockfd) {
fprintf(stderr, "Error opening lock file %s\n", lockname);
-- 
2.6.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 0/7] tools: env: simplify argument parsing

[Andreas Fenkart]

In it's current state paramter parsing is quite hard to
understand since it happens in two places. One is using getopt
at the beginning of main, the second is using adhoc parsing 
where the order of arguments is important.
This patch will parse arguments only in one place using getopt
and store the parsed flags in a global struct.

v2:
- rebased on top of current master

Andreas Fenkart (7):
  tools: env validate: pass values as 0-based array
  tools: env: make parse_aes_key stateless
  tools: env: introduce setenv/printenv argument structs
  tools: env: parse aes key / suppress flag into argument struct
  tools: env: shift optind arguments and fix argument indices
  tools: env: factor out parse_common_args
  tools: env: update usage strings

 common/env_flags.c  |  14 +--
 include/env_flags.h |   2 +-
 tools/env/fw_env.c  | 127 ++--
 tools/env/fw_env.h  |  22 +
 tools/env/fw_env_main.c | 250 ++--
 5 files changed, 238 insertions(+), 177 deletions(-)

-- 
2.6.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 1/7] tools: env validate: pass values as 0-based array

[Andreas Fenkart]

passing argv/argc can produce off-by-one errors

Signed-off-by: Andreas Fenkart <andreas.fenk...@dev.digitalstrom.org>
---
 common/env_flags.c  | 14 +++---
 include/env_flags.h |  2 +-
 tools/env/fw_env.c  | 11 +++
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/common/env_flags.c b/common/env_flags.c
index e682d85..529e371 100644
--- a/common/env_flags.c
+++ b/common/env_flags.c
@@ -373,21 +373,21 @@ int env_flags_validate_varaccess(const char *name, int 
check_mask)
 /*
  * Validate the parameters to "env set" directly
  */
-int env_flags_validate_env_set_params(int argc, char * const argv[])
+int env_flags_validate_env_set_params(char *name, char * const val[], int 
count)
 {
-   if ((argc >= 3) && argv[2] != NULL) {
-   enum env_flags_vartype type = env_flags_get_type(argv[1]);
+   if ((count >= 1) && val[0] != NULL) {
+   enum env_flags_vartype type = env_flags_get_type(name);
 
/*
 * we don't currently check types that need more than
 * one argument
 */
-   if (type != env_flags_vartype_string && argc > 3) {
-   printf("## Error: too many parameters for setting "
-   "\"%s\"\n", argv[1]);
+   if (type != env_flags_vartype_string && count > 1) {
+   printf("## Error: too many parameters for setting 
\"%s\"\n",
+  name);
return -1;
}
-   return env_flags_validate_type(argv[1], argv[2]);
+   return env_flags_validate_type(name, val[0]);
}
/* ok */
return 0;
diff --git a/include/env_flags.h b/include/env_flags.h
index 8823fb9..9e87e1b 100644
--- a/include/env_flags.h
+++ b/include/env_flags.h
@@ -143,7 +143,7 @@ int env_flags_validate_varaccess(const char *name, int 
check_mask);
 /*
  * Validate the parameters passed to "env set" for type compliance
  */
-int env_flags_validate_env_set_params(int argc, char * const argv[]);
+int env_flags_validate_env_set_params(char *name, char *const val[], int 
count);
 
 #else /* !USE_HOSTCC */
 
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index ba11f77..22507f6 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -497,8 +497,9 @@ int fw_setenv(int argc, char *argv[])
 {
int i, rc;
size_t len;
-   char *name;
+   char *name, **valv;
char *value = NULL;
+   int valc;
 
 #ifdef CONFIG_FILE
if (argc >= 2 && strcmp(argv[1], "-c") == 0) {
@@ -542,13 +543,15 @@ int fw_setenv(int argc, char *argv[])
}
 
name = argv[1];
+   valv = argv + 2;
+   valc = argc - 2;
 
-   if (env_flags_validate_env_set_params(argc, argv) < 0)
+   if (env_flags_validate_env_set_params(name, valv, valc) < 0)
return 1;
 
len = 0;
-   for (i = 2; i < argc; ++i) {
-   char *val = argv[i];
+   for (i = 0; i < valc; ++i) {
+   char *val = valv[i];
size_t val_len = strlen(val);
 
if (value)
-- 
2.6.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 3/7] tools: env: introduce setenv/printenv argument structs

[Andreas Fenkart]

goal is to use getopt for all argument parsing instead of adhoc
parsing in fw_getenv/fw_setenv functions

Signed-off-by: Andreas Fenkart <andreas.fenk...@dev.digitalstrom.org>
---
 tools/env/fw_env.h  |   9 
 tools/env/fw_env_main.c | 113 
 2 files changed, 84 insertions(+), 38 deletions(-)

diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
index 60c0517..1a02c46 100644
--- a/tools/env/fw_env.h
+++ b/tools/env/fw_env.h
@@ -54,6 +54,15 @@
"bootm"
 #endif
 
+struct printenv_args {
+};
+extern struct printenv_args printenv_args;
+
+struct setenv_args {
+   char *script_file;
+};
+extern struct setenv_args setenv_args;
+
 extern int   fw_printenv(int argc, char *argv[]);
 extern char *fw_getenv  (char *name);
 extern int fw_setenv  (int argc, char *argv[]);
diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index 234c061..0c9f918 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -45,6 +45,9 @@ static struct option long_options[] = {
{NULL, 0, NULL, 0}
 };
 
+struct printenv_args printenv_args;
+struct setenv_args setenv_args;
+
 void usage(void)
 {
 
@@ -77,31 +80,9 @@ void usage(void)
);
 }
 
-int main(int argc, char *argv[])
+int parse_printenv_args(int argc, char *argv[])
 {
-   char *p;
-   char *cmdname = *argv;
-   char *script_file = NULL;
int c;
-   const char *lockname = "/var/lock/" CMD_PRINTENV ".lock";
-   int lockfd = -1;
-   int retval = EXIT_SUCCESS;
-
-   lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC, 0666);
-   if (-1 == lockfd) {
-   fprintf(stderr, "Error opening lock file %s\n", lockname);
-   return EXIT_FAILURE;
-   }
-
-   if (-1 == flock(lockfd, LOCK_EX)) {
-   fprintf(stderr, "Error locking file %s\n", lockname);
-   close(lockfd);
-   return EXIT_FAILURE;
-   }
-
-   if ((p = strrchr (cmdname, '/')) != NULL) {
-   cmdname = p + 1;
-   }
 
while ((c = getopt_long (argc, argv, "a:c:ns:h",
long_options, NULL)) != EOF) {
@@ -115,40 +96,96 @@ int main(int argc, char *argv[])
case 'n':
/* handled in fw_printenv */
break;
+   case 'h':
+   usage();
+   exit(EXIT_SUCCESS);
+   break;
+   default: /* '?' */
+   usage();
+   exit(EXIT_FAILURE);
+   break;
+   }
+   }
+   return 0;
+}
+
+int parse_setenv_args(int argc, char *argv[])
+{
+   int c;
+
+   while ((c = getopt_long (argc, argv, "a:c:ns:h",
+   long_options, NULL)) != EOF) {
+   switch (c) {
+   case 'a':
+   /* AES key, handled later */
+   break;
+   case 'c':
+   /* handled later */
+   break;
case 's':
-   script_file = optarg;
+   setenv_args.script_file = optarg;
break;
case 'h':
usage();
-   goto exit;
+   exit(EXIT_SUCCESS);
+   break;
default: /* '?' */
-   fprintf(stderr, "Try `%s --help' for more information."
-   "\n", cmdname);
-   retval = EXIT_FAILURE;
-   goto exit;
+   usage();
+   exit(EXIT_FAILURE);
+   break;
}
}
+   return 0;
+}
+
+int main(int argc, char *argv[])
+{
+   char *cmdname = *argv;
+   const char *lockname = "/var/lock/" CMD_PRINTENV ".lock";
+   int lockfd = -1;
+   int retval = EXIT_SUCCESS;
+
+   if (strrchr(cmdname, '/') != NULL)
+   cmdname = strrchr(cmdname, '/') + 1;
+
+   if (strcmp(cmdname, CMD_PRINTENV) == 0) {
+   if (parse_printenv_args(argc, argv))
+   exit(EXIT_FAILURE);
+   } else if (strcmp(cmdname, CMD_SETENV) == 0) {
+   if (parse_setenv_args(argc, argv))
+   exit(EXIT_FAILURE);
+   } else {
+   fprintf(stderr,
+   "Identity crisis - may be called as `%s' or as `%s' but 
not as `%s'\n",
+   CMD_PRINTENV, CMD_SETENV, cmdname);
+   exit(EXIT_FAILURE);
+   }
+
+   lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC, 0666);
+   if (-1 == lockfd) {
+   fprintf(stderr, "Error opening lock file %s\n", lockname);
+   return EXIT_FAILURE;
+   }
+
+ 

[U-Boot] [PATCH v2 5/7] tools: env: shift optind arguments and fix argument indices

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@dev.digitalstrom.org>
---
 tools/env/fw_env.c  | 54 ++---
 tools/env/fw_env_main.c |  4 
 2 files changed, 15 insertions(+), 43 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 2cfff02..b1cf9ae 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -238,22 +238,10 @@ int fw_printenv (int argc, char *argv[])
char *env, *nxt;
int i, rc = 0;
 
-#ifdef CONFIG_FILE
-   if (argc >= 2 && strcmp(argv[1], "-c") == 0) {
-   argv += 2;
-   argc -= 2;
-   }
-#endif
-
-   if (argc >= 2 && strcmp(argv[1], "-a") == 0) {
-   argv += 2;
-   argc -= 2;
-   }
-
if (fw_env_open())
return -1;
 
-   if (argc == 1) {/* Print all env variables  */
+   if (argc == 0) {/* Print all env variables  */
for (env = environment.data; *env; env = nxt + 1) {
for (nxt = env; *nxt; ++nxt) {
if (nxt >= [ENV_SIZE]) {
@@ -268,17 +256,13 @@ int fw_printenv (int argc, char *argv[])
return 0;
}
 
-   if (strcmp (argv[1], "-n") == 0) {
-   ++argv;
-   --argc;
-   if (argc != 2) {
-   fprintf (stderr, "## Error: "
-   "`-n' option requires exactly one argument\n");
-   return -1;
-   }
+   if (printenv_args.name_suppress && argc != 1) {
+   fprintf(stderr,
+   "## Error: `-n' option requires exactly one 
argument\n");
+   return -1;
}
 
-   for (i = 1; i < argc; ++i) {/* print single env variables   */
+   for (i = 0; i < argc; ++i) {/* print single env variables   */
char *name = argv[i];
char *val = NULL;
 
@@ -475,24 +459,8 @@ int fw_setenv(int argc, char *argv[])
char *value = NULL;
int valc;
 
-#ifdef CONFIG_FILE
-   if (argc >= 2 && strcmp(argv[1], "-c") == 0) {
-   argv += 2;
-   argc -= 2;
-   }
-#endif
-
-   if (argc < 2) {
-   errno = EINVAL;
-   return -1;
-   }
-
-   if (strcmp(argv[1], "-a") == 0) {
-   argv += 2;
-   argc -= 2;
-   }
-
-   if (argc < 2) {
+   if (argc < 1) {
+   fprintf(stderr, "## Error: variable name missing\n");
errno = EINVAL;
return -1;
}
@@ -502,9 +470,9 @@ int fw_setenv(int argc, char *argv[])
return -1;
}
 
-   name = argv[1];
-   valv = argv + 2;
-   valc = argc - 2;
+   name = argv[0];
+   valv = argv + 1;
+   valc = argc - 1;
 
if (env_flags_validate_env_set_params(name, valv, valc) < 0)
return 1;
diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index b68f1bf..daf4688 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -182,6 +182,10 @@ int main(int argc, char *argv[])
exit(EXIT_FAILURE);
}
 
+   /* shift parsed flags, jump to non-option arguments */
+   argc -= optind;
+   argv += optind;
+
lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC, 0666);
if (-1 == lockfd) {
fprintf(stderr, "Error opening lock file %s\n", lockname);
-- 
2.6.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 4/7] tools: env: parse aes key / suppress flag into argument struct

[Andreas Fenkart]

disabled original parsing, but not yet removed since the
argument indexing needs to be fixed

Signed-off-by: Andreas Fenkart <andreas.fenk...@dev.digitalstrom.org>
---
 tools/env/fw_env.c  | 64 ++---
 tools/env/fw_env.h  | 13 ++
 tools/env/fw_env_main.c | 31 
 3 files changed, 51 insertions(+), 57 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 5b76b74..2cfff02 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -33,8 +33,6 @@
 
 #include "fw_env.h"
 
-#include 
-
 #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
 
 #define WHITESPACE(c) ((c == '\t') || (c == ' '))
@@ -104,9 +102,6 @@ static struct environment environment = {
.flag_scheme = FLAG_NONE,
 };
 
-/* Is AES encryption used? */
-static int aes_flag;
-static uint8_t aes_key[AES_KEY_LENGTH] = { 0 };
 static int env_aes_cbc_crypt(char *data, const int enc);
 
 static int HaveRedundEnv = 0;
@@ -124,7 +119,6 @@ static int parse_config (void);
 
 #if defined(CONFIG_FILE)
 static int get_config (char *);
-static char *config_file = CONFIG_FILE;
 #endif
 static inline ulong getenvsize (void)
 {
@@ -133,7 +127,7 @@ static inline ulong getenvsize (void)
if (HaveRedundEnv)
rc -= sizeof (char);
 
-   if (aes_flag)
+   if (common_args.aes_flag)
rc &= ~(AES_KEY_LENGTH - 1);
 
return rc;
@@ -207,7 +201,7 @@ char *fw_getdefenv(char *name)
return NULL;
 }
 
-static int parse_aes_key(char *key, uint8_t *bin_key)
+int parse_aes_key(char *key, uint8_t *bin_key)
 {
char tmp[5] = { '0', 'x', 0, 0, 0 };
unsigned long ul;
@@ -242,32 +236,16 @@ static int parse_aes_key(char *key, uint8_t *bin_key)
 int fw_printenv (int argc, char *argv[])
 {
char *env, *nxt;
-   int i, n_flag;
-   int rc = 0;
+   int i, rc = 0;
 
 #ifdef CONFIG_FILE
if (argc >= 2 && strcmp(argv[1], "-c") == 0) {
-   if (argc < 3) {
-   fprintf(stderr,
-   "## Error: '-c' option requires the config file 
to use\n");
-   return -1;
-   }
-   config_file = argv[2];
argv += 2;
argc -= 2;
}
 #endif
 
if (argc >= 2 && strcmp(argv[1], "-a") == 0) {
-   if (argc < 3) {
-   fprintf(stderr,
-   "## Error: '-a' option requires AES key\n");
-   return -1;
-   }
-   rc = parse_aes_key(argv[2], aes_key);
-   if (rc)
-   return rc;
-   aes_flag = 1;
argv += 2;
argc -= 2;
}
@@ -291,7 +269,6 @@ int fw_printenv (int argc, char *argv[])
}
 
if (strcmp (argv[1], "-n") == 0) {
-   n_flag = 1;
++argv;
--argc;
if (argc != 2) {
@@ -299,8 +276,6 @@ int fw_printenv (int argc, char *argv[])
"`-n' option requires exactly one argument\n");
return -1;
}
-   } else {
-   n_flag = 0;
}
 
for (i = 1; i < argc; ++i) {/* print single env variables   */
@@ -318,7 +293,7 @@ int fw_printenv (int argc, char *argv[])
}
val = envmatch (name, env);
if (val) {
-   if (!n_flag) {
+   if (!printenv_args.name_suppress) {
fputs (name, stdout);
putc ('=', stdout);
}
@@ -338,7 +313,7 @@ int fw_printenv (int argc, char *argv[])
 int fw_env_close(void)
 {
int ret;
-   if (aes_flag) {
+   if (common_args.aes_flag) {
ret = env_aes_cbc_crypt(environment.data, 1);
if (ret) {
fprintf(stderr,
@@ -494,7 +469,7 @@ int fw_env_write(char *name, char *value)
  */
 int fw_setenv(int argc, char *argv[])
 {
-   int i, rc;
+   int i;
size_t len;
char *name, **valv;
char *value = NULL;
@@ -502,12 +477,6 @@ int fw_setenv(int argc, char *argv[])
 
 #ifdef CONFIG_FILE
if (argc >= 2 && strcmp(argv[1], "-c") == 0) {
-   if (argc < 3) {
-   fprintf(stderr,
-   "## Error: '-c' option requires the config file 
to use\n");
-   return -1;
-   }
-   config_file = argv[2];
argv += 2;
argc -= 2;
}
@@ -519,15 +488,6 @@ int fw_setenv(int argc, char *argv[])
}
 
if (strcmp(argv[1], "-a

[U-Boot] [PATCH v2 2/7] tools: env: make parse_aes_key stateless

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@dev.digitalstrom.org>
---
 tools/env/fw_env.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 22507f6..5b76b74 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -207,7 +207,7 @@ char *fw_getdefenv(char *name)
return NULL;
 }
 
-static int parse_aes_key(char *key)
+static int parse_aes_key(char *key, uint8_t *bin_key)
 {
char tmp[5] = { '0', 'x', 0, 0, 0 };
unsigned long ul;
@@ -229,11 +229,9 @@ static int parse_aes_key(char *key)
"## Error: '-a' option requires valid AES 
key\n");
return -1;
}
-   aes_key[i] = ul & 0xff;
+   bin_key[i] = ul & 0xff;
key += 2;
}
-   aes_flag = 1;
-
return 0;
 }
 
@@ -266,9 +264,10 @@ int fw_printenv (int argc, char *argv[])
"## Error: '-a' option requires AES key\n");
return -1;
}
-   rc = parse_aes_key(argv[2]);
+   rc = parse_aes_key(argv[2], aes_key);
if (rc)
return rc;
+   aes_flag = 1;
argv += 2;
argc -= 2;
}
@@ -525,9 +524,10 @@ int fw_setenv(int argc, char *argv[])
"## Error: '-a' option requires AES key\n");
return -1;
}
-   rc = parse_aes_key(argv[2]);
+   rc = parse_aes_key(argv[2], aes_key);
if (rc)
return rc;
+   aes_flag = 1;
argv += 2;
argc -= 2;
}
-- 
2.6.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 6/7] tools: env: factor out parse_common_args

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@dev.digitalstrom.org>
---
 tools/env/fw_env_main.c | 63 +
 1 file changed, 38 insertions(+), 25 deletions(-)

diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index daf4688..e7ba95e 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -81,7 +81,7 @@ void usage(void)
);
 }
 
-int parse_printenv_args(int argc, char *argv[])
+static void parse_common_args(int argc, char *argv[])
 {
int c;
 
@@ -89,27 +89,53 @@ int parse_printenv_args(int argc, char *argv[])
common_args.config_file = CONFIG_FILE;
 #endif
 
-   while ((c = getopt_long (argc, argv, "a:c:ns:h",
-   long_options, NULL)) != EOF) {
+   while ((c = getopt_long(argc, argv, ":a:c:h", long_options, NULL)) !=
+  EOF) {
switch (c) {
case 'a':
if (parse_aes_key(optarg, common_args.aes_key)) {
fprintf(stderr, "AES key parse error\n");
-   return EXIT_FAILURE;
+   exit(EXIT_FAILURE);
}
common_args.aes_flag = 1;
break;
 #ifdef CONFIG_FILE
case 'c':
+   printf("config file\n");
common_args.config_file = optarg;
break;
 #endif
+   case 'h':
+   usage();
+   exit(EXIT_SUCCESS);
+   break;
+   default:
+   /* ignore unknown options */
+   break;
+   }
+   }
+
+   /* Reset getopt for the next pass. */
+   opterr = 1;
+   optind = 1;
+}
+
+int parse_printenv_args(int argc, char *argv[])
+{
+   int c;
+
+   parse_common_args(argc, argv);
+
+   while ((c = getopt_long(argc, argv, "a:c:ns:h", long_options, NULL)) !=
+  EOF) {
+   switch (c) {
case 'n':
printenv_args.name_suppress = 1;
break;
+   case 'a':
+   case 'c':
case 'h':
-   usage();
-   exit(EXIT_SUCCESS);
+   /* ignore common options */
break;
default: /* '?' */
usage();
@@ -124,31 +150,18 @@ int parse_setenv_args(int argc, char *argv[])
 {
int c;
 
-#ifdef CONFIG_FILE
-   common_args.config_file = CONFIG_FILE;
-#endif
+   parse_common_args(argc, argv);
 
-   while ((c = getopt_long (argc, argv, "a:c:ns:h",
-   long_options, NULL)) != EOF) {
+   while ((c = getopt_long(argc, argv, "a:c:ns:h", long_options, NULL)) !=
+  EOF) {
switch (c) {
-   case 'a':
-   if (parse_aes_key(optarg, common_args.aes_key)) {
-   fprintf(stderr, "AES key parse error\n");
-   return EXIT_FAILURE;
-   }
-   common_args.aes_flag = 1;
-   break;
-#ifdef CONFIG_FILE
-   case 'c':
-   common_args.config_file = optarg;
-   break;
-#endif
case 's':
setenv_args.script_file = optarg;
break;
+   case 'a':
+   case 'c':
case 'h':
-   usage();
-   exit(EXIT_SUCCESS);
+   /* ignore common options */
break;
default: /* '?' */
usage();
-- 
2.6.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 7/7] tools: env: update usage strings

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@dev.digitalstrom.org>
---
 tools/env/fw_env_main.c | 117 +++-
 1 file changed, 75 insertions(+), 42 deletions(-)

diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index e7ba95e..79273ce 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -36,12 +36,16 @@
 #include 
 #include "fw_env.h"
 
-#defineCMD_PRINTENV"fw_printenv"
+#define CMD_PRINTENV   "fw_printenv"
 #define CMD_SETENV "fw_setenv"
+static int do_printenv;
 
 static struct option long_options[] = {
-   {"script", required_argument, NULL, 's'},
+   {"aes", required_argument, NULL, 'a'},
+   {"config", required_argument, NULL, 'c'},
{"help", no_argument, NULL, 'h'},
+   {"script", required_argument, NULL, 's'},
+   {"noheader", required_argument, NULL, 'n'},
{NULL, 0, NULL, 0}
 };
 
@@ -49,36 +53,58 @@ struct common_args common_args;
 struct printenv_args printenv_args;
 struct setenv_args setenv_args;
 
-void usage(void)
+void usage_printenv(void)
 {
 
-   fprintf(stderr, "fw_printenv/fw_setenv, "
-   "a command line interface to U-Boot environment\n\n"
-#ifndef CONFIG_FILE
-   "usage:\tfw_printenv [-a key] [-n] [variable name]\n"
-   "\tfw_setenv [-a key] [variable name] [variable value]\n"
-#else
-   "usage:\tfw_printenv [-c /my/fw_env.config] [-a key] [-n] 
[variable name]\n"
-   "\tfw_setenv [-c /my/fw_env.config] [-a key] [variable name] 
[variable value]\n"
+   fprintf(stderr,
+   "Usage: fw_printenv [OPTIONS]... [VARIABLE]...\n"
+   "Print variables from U-Boot environment\n"
+   "\n"
+   " -h, --help   print this help.\n"
+#ifdef CONFIG_ENV_AES
+   " -a, --aesaes key to access environment\n"
 #endif
-   "\tfw_setenv -s [ file ]\n"
-   "\tfw_setenv -s - < [ file ]\n\n"
-   "The file passed as argument contains only pairs "
-   "name / value\n"
-   "Example:\n"
-   "# Any line starting with # is treated as comment\n"
+#ifdef CONFIG_FILE
+   " -c, --config configuration file, default:" 
CONFIG_FILE "\n"
+#endif
+   " -n, --noheader   do not repeat variable name in output\n"
+   "\n");
+}
+
+void usage_setenv(void)
+{
+   fprintf(stderr,
+   "Usage: fw_setenv [OPTIONS]... [VARIABLE]...\n"
+   "Modify variables in U-Boot environment\n"
+   "\n"
+   " -h, --help   print this help.\n"
+#ifdef CONFIG_ENV_AES
+   " -a, --aesaes key to access environment\n"
+#endif
+#ifdef CONFIG_FILE
+   " -c, --config configuration file, default:" 
CONFIG_FILE "\n"
+#endif
+   " -s, --script batch mode to minimize writes\n"
+   "\n"
+   "Examples:\n"
+   "  fw_setenv foo bar   set variable foo equal bar\n"
+   "  fw_setenv foo   clear variable foo\n"
+   "  fw_setenv --script file run batch script\n"
"\n"
-   "\t  netdev eth0\n"
-   "\t  kernel_addr40\n"
-   "\t  var1\n"
-   "\t  var2  The quick brown fox jumps over the "
-   "lazy dog\n"
+   "Script Syntax:\n"
+   "  key [space] value\n"
+   "  lines starting with '#' are treated as commment\n"
"\n"
-   "A variable without value will be dropped. It is possible\n"
-   "to put any number of spaces between the fields, but any\n"
-   "space inside the value is treated as part of the value "
-   "itself.\n\n"
-   );
+   "  A variable without value will be deleted. Any number of 
spaces are\n"
+   "  allowed between key and value. Space inside of the value is 
treated\n"
+   "  as part of the value itself.\n"
+   "\n"
+   "Script Example:\n"
+   "  netdev eth0\n"
+   "  kernel_addr40\n"
+   "  fooempty 

[U-Boot] [PATCH 5/5] tools: env: shift optind arguments and fix argument indices

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@dev.digitalstrom.org>
---
 tools/env/fw_env.c  | 40 +++-
 tools/env/fw_env_main.c |  4 
 2 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 32bb3aa..a89bde5 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -236,15 +236,10 @@ int fw_printenv (int argc, char *argv[])
char *env, *nxt;
int i, rc = 0;
 
-   if (argc >= 2 && strcmp(argv[1], "-a") == 0) {
-   argv += 2;
-   argc -= 2;
-   }
-
if (fw_env_open())
return -1;
 
-   if (argc == 1) {/* Print all env variables  */
+   if (argc == 0) {/* Print all env variables  */
for (env = environment.data; *env; env = nxt + 1) {
for (nxt = env; *nxt; ++nxt) {
if (nxt >= [ENV_SIZE]) {
@@ -259,17 +254,13 @@ int fw_printenv (int argc, char *argv[])
return 0;
}
 
-   if (strcmp (argv[1], "-n") == 0) {
-   ++argv;
-   --argc;
-   if (argc != 2) {
-   fprintf (stderr, "## Error: "
-   "`-n' option requires exactly one argument\n");
-   return -1;
-   }
+   if (printenv_args.name_suppress && argc != 1) {
+   fprintf(stderr,
+   "## Error: `-n' option requires exactly one 
argument\n");
+   return -1;
}
 
-   for (i = 1; i < argc; ++i) {/* print single env variables   */
+   for (i = 0; i < argc; ++i) {/* print single env variables   */
char *name = argv[i];
char *val = NULL;
 
@@ -466,17 +457,8 @@ int fw_setenv(int argc, char *argv[])
char *value = NULL;
int valc;
 
-   if (argc < 2) {
-   errno = EINVAL;
-   return -1;
-   }
-
-   if (strcmp(argv[1], "-a") == 0) {
-   argv += 2;
-   argc -= 2;
-   }
-
-   if (argc < 2) {
+   if (argc < 1) {
+   fprintf(stderr, "## Error: variable name missing\n");
errno = EINVAL;
return -1;
}
@@ -486,9 +468,9 @@ int fw_setenv(int argc, char *argv[])
return -1;
}
 
-   name = argv[1];
-   valv = argv + 2;
-   valc = argc - 2;
+   name = argv[0];
+   valv = argv + 1;
+   valc = argc - 1;
 
if (env_flags_validate_env_set_params(name, valv, valc) < 0)
return 1;
diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index c828fd0..c20325f 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -158,6 +158,10 @@ int main(int argc, char *argv[])
exit(EXIT_FAILURE);
}
 
+   /* shift parsed flags, jump to non-option arguments */
+   argc -= optind;
+   argv += optind;
+
lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC, 0666);
if (-1 == lockfd) {
fprintf(stderr, "Error opening lock file %s\n", lockname);
-- 
2.6.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 4/5] tools: env: parse aes key / suppress flag into argument struct

[Andreas Fenkart]

disabled original parsing, but not yet removed since the
argument indexing needs to be fixed

Signed-off-by: Andreas Fenkart <andreas.fenk...@dev.digitalstrom.org>
---
 tools/env/fw_env.c  | 45 +
 tools/env/fw_env.h  | 12 
 tools/env/fw_env_main.c | 15 ---
 3 files changed, 33 insertions(+), 39 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index f1dea8b..32bb3aa 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -31,8 +31,6 @@
 
 #include "fw_env.h"
 
-#include 
-
 #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
 
 #define WHITESPACE(c) ((c == '\t') || (c == ' '))
@@ -102,9 +100,6 @@ static struct environment environment = {
.flag_scheme = FLAG_NONE,
 };
 
-/* Is AES encryption used? */
-static int aes_flag;
-static uint8_t aes_key[AES_KEY_LENGTH] = { 0 };
 static int env_aes_cbc_crypt(char *data, const int enc);
 
 static int HaveRedundEnv = 0;
@@ -130,7 +125,7 @@ static inline ulong getenvsize (void)
if (HaveRedundEnv)
rc -= sizeof (char);
 
-   if (aes_flag)
+   if (common_args.aes_flag)
rc &= ~(AES_KEY_LENGTH - 1);
 
return rc;
@@ -204,7 +199,7 @@ char *fw_getdefenv(char *name)
return NULL;
 }
 
-static int parse_aes_key(char *key, uint8_t *bin_key)
+int parse_aes_key(char *key, uint8_t *bin_key)
 {
char tmp[5] = { '0', 'x', 0, 0, 0 };
unsigned long ul;
@@ -239,19 +234,9 @@ static int parse_aes_key(char *key, uint8_t *bin_key)
 int fw_printenv (int argc, char *argv[])
 {
char *env, *nxt;
-   int i, n_flag;
-   int rc = 0;
+   int i, rc = 0;
 
if (argc >= 2 && strcmp(argv[1], "-a") == 0) {
-   if (argc < 3) {
-   fprintf(stderr,
-   "## Error: '-a' option requires AES key\n");
-   return -1;
-   }
-   rc = parse_aes_key(argv[2], aes_key);
-   if (rc)
-   return rc;
-   aes_flag = 1;
argv += 2;
argc -= 2;
}
@@ -275,7 +260,6 @@ int fw_printenv (int argc, char *argv[])
}
 
if (strcmp (argv[1], "-n") == 0) {
-   n_flag = 1;
++argv;
--argc;
if (argc != 2) {
@@ -283,8 +267,6 @@ int fw_printenv (int argc, char *argv[])
"`-n' option requires exactly one argument\n");
return -1;
}
-   } else {
-   n_flag = 0;
}
 
for (i = 1; i < argc; ++i) {/* print single env variables   */
@@ -302,7 +284,7 @@ int fw_printenv (int argc, char *argv[])
}
val = envmatch (name, env);
if (val) {
-   if (!n_flag) {
+   if (!printenv_args.name_suppress) {
fputs (name, stdout);
putc ('=', stdout);
}
@@ -322,7 +304,7 @@ int fw_printenv (int argc, char *argv[])
 int fw_env_close(void)
 {
int ret;
-   if (aes_flag) {
+   if (common_args.aes_flag) {
ret = env_aes_cbc_crypt(environment.data, 1);
if (ret) {
fprintf(stderr,
@@ -478,7 +460,7 @@ int fw_env_write(char *name, char *value)
  */
 int fw_setenv(int argc, char *argv[])
 {
-   int i, rc;
+   int i;
size_t len;
char *name, **valv;
char *value = NULL;
@@ -490,15 +472,6 @@ int fw_setenv(int argc, char *argv[])
}
 
if (strcmp(argv[1], "-a") == 0) {
-   if (argc < 3) {
-   fprintf(stderr,
-   "## Error: '-a' option requires AES key\n");
-   return -1;
-   }
-   rc = parse_aes_key(argv[2], aes_key);
-   if (rc)
-   return rc;
-   aes_flag = 1;
argv += 2;
argc -= 2;
}
@@ -996,7 +969,7 @@ static int env_aes_cbc_crypt(char *payload, const int enc)
uint32_t aes_blocks;
 
/* First we expand the key. */
-   aes_expand_key(aes_key, key_exp);
+   aes_expand_key(common_args.aes_key, key_exp);
 
/* Calculate the number of AES blocks to encrypt. */
aes_blocks = DIV_ROUND_UP(len, AES_KEY_LENGTH);
@@ -1224,7 +1197,7 @@ int fw_env_open(void)
 
crc0 = crc32 (0, (uint8_t *) environment.data, ENV_SIZE);
 
-   if (aes_flag) {
+   if (common_args.aes_flag) {
ret = env_aes_cbc_crypt(environment.data, 0);
if (ret)
return ret;
@@ -1281,7 +1254,7 @@ int fw_env_open(void)
 
   

[U-Boot] [PATCH 3/5] tools: env: introduce setenv/printenv argument structs

[Andreas Fenkart]

goal is to use getopt for all argument parsing instead of adhoc
parsing in fw_getenv/fw_setenv functions

Signed-off-by: Andreas Fenkart <andreas.fenk...@dev.digitalstrom.org>
---
 tools/env/fw_env.h  |   9 
 tools/env/fw_env_main.c | 111 +++-
 2 files changed, 81 insertions(+), 39 deletions(-)

diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
index aff471b..4ac7de4 100644
--- a/tools/env/fw_env.h
+++ b/tools/env/fw_env.h
@@ -52,6 +52,15 @@
"bootm"
 #endif
 
+struct printenv_args {
+};
+extern struct printenv_args printenv_args;
+
+struct setenv_args {
+   char *script_file;
+};
+extern struct setenv_args setenv_args;
+
 extern int   fw_printenv(int argc, char *argv[]);
 extern char *fw_getenv  (char *name);
 extern int fw_setenv  (int argc, char *argv[]);
diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index ce50d58..f45c94a 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -45,6 +45,9 @@ static struct option long_options[] = {
{NULL, 0, NULL, 0}
 };
 
+struct printenv_args printenv_args;
+struct setenv_args setenv_args;
+
 void usage(void)
 {
 
@@ -72,33 +75,11 @@ void usage(void)
);
 }
 
-int main(int argc, char *argv[])
+int parse_printenv_args(int argc, char *argv[])
 {
-   char *p;
-   char *cmdname = *argv;
-   char *script_file = NULL;
int c;
-   const char *lockname = "/var/lock/" CMD_PRINTENV ".lock";
-   int lockfd = -1;
-   int retval = EXIT_SUCCESS;
-
-   lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC, 0666);
-   if (-1 == lockfd) {
-   fprintf(stderr, "Error opening lock file %s\n", lockname);
-   return EXIT_FAILURE;
-   }
-
-   if (-1 == flock(lockfd, LOCK_EX)) {
-   fprintf(stderr, "Error locking file %s\n", lockname);
-   close(lockfd);
-   return EXIT_FAILURE;
-   }
 
-   if ((p = strrchr (cmdname, '/')) != NULL) {
-   cmdname = p + 1;
-   }
-
-   while ((c = getopt_long (argc, argv, "a:ns:h",
+   while ((c = getopt_long (argc, argv, "a:nh",
long_options, NULL)) != EOF) {
switch (c) {
case 'a':
@@ -107,40 +88,92 @@ int main(int argc, char *argv[])
case 'n':
/* handled in fw_printenv */
break;
+   case 'h':
+   usage();
+   exit(EXIT_SUCCESS);
+   break;
+   default: /* '?' */
+   usage();
+   exit(EXIT_FAILURE);
+   break;
+   }
+   }
+   return 0;
+}
+
+int parse_setenv_args(int argc, char *argv[])
+{
+   int c;
+   while ((c = getopt_long (argc, argv, "a:s:h",
+   long_options, NULL)) != EOF) {
+   switch (c) {
+   case 'a':
+   /* AES key, handled later */
+   break;
case 's':
-   script_file = optarg;
+   setenv_args.script_file = optarg;
break;
case 'h':
usage();
-   goto exit;
+   exit(EXIT_SUCCESS);
+   break;
default: /* '?' */
-   fprintf(stderr, "Try `%s --help' for more information."
-   "\n", cmdname);
-   retval = EXIT_FAILURE;
-   goto exit;
+   usage();
+   exit(EXIT_FAILURE);
+   break;
}
}
+   return 0;
+}
+
+int main(int argc, char *argv[])
+{
+   char *cmdname = *argv;
+   const char *lockname = "/var/lock/" CMD_PRINTENV ".lock";
+   int lockfd = -1;
+   int retval = EXIT_SUCCESS;
+
+   if (strrchr(cmdname, '/') != NULL)
+   cmdname = strrchr(cmdname, '/') + 1;
+
+   if (strcmp(cmdname, CMD_PRINTENV) == 0) {
+   if (parse_printenv_args(argc, argv))
+   exit(EXIT_FAILURE);
+   } else if (strcmp(cmdname, CMD_SETENV) == 0) {
+   if (parse_setenv_args(argc, argv))
+   exit(EXIT_FAILURE);
+   } else {
+   fprintf(stderr,
+   "Identity crisis - may be called as `%s' or as `%s' but 
not as `%s'\n",
+   CMD_PRINTENV, CMD_SETENV, cmdname);
+   exit(EXIT_FAILURE);
+   }
+
+   lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC, 0666);
+   if (-1 == lockfd) {
+   fprintf(stderr, "Error opening lock file %s\n", lockname);
+   return EXIT_FAILURE;
+

[U-Boot] [PATCH 1/5] tools: env validate: pass values as 0-based array

[Andreas Fenkart]

passing argv/argc can produce off-by-one errors

Signed-off-by: Andreas Fenkart <andreas.fenk...@dev.digitalstrom.org>
---
 common/env_flags.c  | 14 +++---
 include/env_flags.h |  2 +-
 tools/env/fw_env.c  | 11 +++
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/common/env_flags.c b/common/env_flags.c
index 985f92e..fb5ac40 100644
--- a/common/env_flags.c
+++ b/common/env_flags.c
@@ -359,21 +359,21 @@ int env_flags_validate_varaccess(const char *name, int 
check_mask)
 /*
  * Validate the parameters to "env set" directly
  */
-int env_flags_validate_env_set_params(int argc, char * const argv[])
+int env_flags_validate_env_set_params(char *name, char * const val[], int 
count)
 {
-   if ((argc >= 3) && argv[2] != NULL) {
-   enum env_flags_vartype type = env_flags_get_type(argv[1]);
+   if ((count >= 1) && val[0] != NULL) {
+   enum env_flags_vartype type = env_flags_get_type(name);
 
/*
 * we don't currently check types that need more than
 * one argument
 */
-   if (type != env_flags_vartype_string && argc > 3) {
-   printf("## Error: too many parameters for setting "
-   "\"%s\"\n", argv[1]);
+   if (type != env_flags_vartype_string && count > 1) {
+   printf("## Error: too many parameters for setting 
\"%s\"\n",
+  name);
return -1;
}
-   return env_flags_validate_type(argv[1], argv[2]);
+   return env_flags_validate_type(name, val[0]);
}
/* ok */
return 0;
diff --git a/include/env_flags.h b/include/env_flags.h
index 3ef6311..b153a71 100644
--- a/include/env_flags.h
+++ b/include/env_flags.h
@@ -121,7 +121,7 @@ int env_flags_validate_varaccess(const char *name, int 
check_mask);
 /*
  * Validate the parameters passed to "env set" for type compliance
  */
-int env_flags_validate_env_set_params(int argc, char * const argv[]);
+int env_flags_validate_env_set_params(char *name, char *const val[], int 
count);
 
 #else /* !USE_HOSTCC */
 
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 1173eea..bcf3756 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -481,8 +481,9 @@ int fw_setenv(int argc, char *argv[])
 {
int i, rc;
size_t len;
-   char *name;
+   char *name, **valv;
char *value = NULL;
+   int valc;
 
if (argc < 2) {
errno = EINVAL;
@@ -513,13 +514,15 @@ int fw_setenv(int argc, char *argv[])
}
 
name = argv[1];
+   valv = argv + 2;
+   valc = argc - 2;
 
-   if (env_flags_validate_env_set_params(argc, argv) < 0)
+   if (env_flags_validate_env_set_params(name, valv, valc) < 0)
return 1;
 
len = 0;
-   for (i = 2; i < argc; ++i) {
-   char *val = argv[i];
+   for (i = 0; i < valc; ++i) {
+   char *val = valv[i];
size_t val_len = strlen(val);
 
if (value)
-- 
2.6.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 0/5] tools: env: simplify argument parsing

[Andreas Fenkart]

I want to add -c config_file parameter to fw_setenv/fw_printenv,
so I can switch between old/new u-boot environment after u-boot
upgrade.
In it's current state paramter parsing is quite hard to
understand since it happens in two places. One is using getopt
at the beginning of main, the second is using adhoc parsing 
where the order of arguments is important.
This patch will parse arguments only in one place using getopt
and store the parsed flags in a global struct.

Andreas Fenkart (5):
  tools: env validate: pass values as 0-based array
  tools: env: make parse_aes_key stateless
  tools: env: introduce setenv/printenv argument structs
  tools: env: parse aes key / suppress flag into argument struct
  tools: env: shift optind arguments and fix argument indices

 common/env_flags.c  |  14 +++---
 include/env_flags.h |   2 +-
 tools/env/fw_env.c  |  94 ++--
 tools/env/fw_env.h  |  21 
 tools/env/fw_env_main.c | 124 +---
 5 files changed, 140 insertions(+), 115 deletions(-)

-- 
2.6.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/5] tools: env: make parse_aes_key stateless

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenk...@dev.digitalstrom.org>
---
 tools/env/fw_env.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index bcf3756..f1dea8b 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -204,7 +204,7 @@ char *fw_getdefenv(char *name)
return NULL;
 }
 
-static int parse_aes_key(char *key)
+static int parse_aes_key(char *key, uint8_t *bin_key)
 {
char tmp[5] = { '0', 'x', 0, 0, 0 };
unsigned long ul;
@@ -226,11 +226,9 @@ static int parse_aes_key(char *key)
"## Error: '-a' option requires valid AES 
key\n");
return -1;
}
-   aes_key[i] = ul & 0xff;
+   bin_key[i] = ul & 0xff;
key += 2;
}
-   aes_flag = 1;
-
return 0;
 }
 
@@ -250,9 +248,10 @@ int fw_printenv (int argc, char *argv[])
"## Error: '-a' option requires AES key\n");
return -1;
}
-   rc = parse_aes_key(argv[2]);
+   rc = parse_aes_key(argv[2], aes_key);
if (rc)
return rc;
+   aes_flag = 1;
argv += 2;
argc -= 2;
}
@@ -496,9 +495,10 @@ int fw_setenv(int argc, char *argv[])
"## Error: '-a' option requires AES key\n");
return -1;
}
-   rc = parse_aes_key(argv[2]);
+   rc = parse_aes_key(argv[2], aes_key);
if (rc)
return rc;
+   aes_flag = 1;
argv += 2;
argc -= 2;
}
-- 
2.6.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot