All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file
@ 2011-08-05 14:49 David Wagner
  2011-08-05 15:23 ` Thomas Petazzoni
                   ` (10 more replies)
  0 siblings, 11 replies; 31+ messages in thread
From: David Wagner @ 2011-08-05 14:49 UTC (permalink / raw)
  To: u-boot

This tool takes a key=value configuration file (same as would a `printenv' show)
and generates the corresponding environnment image, ready to be flashed.

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/Makefile     |    5 ++
 tools/mkenvimage.c |  157 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 162 insertions(+), 0 deletions(-)
 create mode 100644 tools/mkenvimage.c

diff --git a/tools/Makefile b/tools/Makefile
index e813e1d..db8522f 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -69,6 +69,7 @@ BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX)
 BIN_FILES-y += mkimage$(SFX)
 BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX)
 BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
+BIN_FILES-y += mkenvimage$(SFX)
 
 # Source files which exist outside the tools directory
 EXT_OBJ_FILES-$(CONFIG_BUILD_ENVCRC) += common/env_embedded.o
@@ -93,6 +94,7 @@ OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o
 NOPED_OBJ_FILES-y += os_support.o
 OBJ_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1.o
 NOPED_OBJ_FILES-y += ublimage.o
+NOPED_OBJ_FILES-y += mkenvimage.o
 
 # Don't build by default
 #ifeq ($(ARCH),ppc)
@@ -171,6 +173,9 @@ $(obj)bmp_logo$(SFX):	$(obj)bmp_logo.o
 $(obj)envcrc$(SFX):	$(obj)crc32.o $(obj)env_embedded.o $(obj)envcrc.o $(obj)sha1.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
 
+$(obj)mkenvimage$(SFX):	$(obj)crc32.o $(obj)mkenvimage.o
+	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
+
 $(obj)gen_eth_addr$(SFX):	$(obj)gen_eth_addr.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
 	$(HOSTSTRIP) $@
diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
new file mode 100644
index 0000000..3fba5ea
--- /dev/null
+++ b/tools/mkenvimage.c
@@ -0,0 +1,157 @@
+/*
+ * (C) Copyright 2011 Free Electrons
+ * David Wagner <david.wagner@free-electrons.com>
+ * 
+ * Inspired from envcrc.c:
+ * (C) Copyright 2001
+ * Paolo Scaffardi, AIRVENT SAM s.p.a - RIMINI(ITALY), arsenio at tin.it
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <errno.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <endian.h>
+
+extern uint32_t crc32 (uint32_t, const unsigned char *, unsigned int);
+
+#define CRC_SIZE sizeof(uint32_t)
+
+static void usage(void)
+{
+	printf("envcrc [-h] [-r] [-b] -s <envrionnment partition size> -o <output> "
+	       "<input file>\n"
+	       "\n"
+	       "\tThe input file is in format:\n"
+	       "\t\tkey1=value1\n"
+	       "\t\tkey2=value2\n"
+	       "\t\t...\n"
+	       "\t-r : the environnment is redundand\n"
+	       "\t-b : the target is big endian (default is little endian)\n");
+
+}
+
+static int make_binary_config(FILE* txt_file, unsigned char *envptr, int envsize)
+{
+	int i;
+	int ret;
+
+	ret = fread(envptr, envsize, 1, txt_file);
+	for (i = 0 ; i < envsize ; i++)
+		if (envptr[i] == '\n')
+			envptr[i] = '\0';
+
+	return 0;
+}
+
+int main(int argc, char **argv)
+{
+	uint32_t crc;
+	char *txt_filename = NULL, *bin_filename = NULL;
+	FILE *txt_file, *bin_file;
+	unsigned char *dataptr, *envptr;
+	unsigned int envsize, datasize = 0;
+	int bigendian = 0;
+	int redundant = 0;
+
+	int option;
+	int ret = EXIT_SUCCESS;
+
+	opterr = 0;
+
+
+	/* Parse the cmdline */
+	while ((option = getopt(argc, argv, "s:o:rbh")) != -1)
+		switch (option)
+		{
+		case 's':
+			datasize = atoi(optarg);
+			break;
+		case 'o':
+			bin_filename = strdup(optarg);
+			if (!bin_filename)
+				return ENOMEM;
+			break;
+		case 'r':
+			redundant = 1;
+			break;
+		case 'b':
+			bigendian = 1;
+			break;
+		case 'h':
+			usage();
+			return EXIT_SUCCESS;
+		default:
+			if (bin_filename)
+				free(bin_filename);
+			fprintf(stderr, "Wrong option\n");
+			usage();
+			return EXIT_FAILURE;
+		}
+
+	if (datasize == 0) {
+		printf("Please specify the size of the envrionnment partition.\n");
+		usage();
+		ret = EXIT_FAILURE;
+		goto out;
+	}
+	
+
+	txt_filename = strdup(argv[optind]);
+	if (!txt_filename) {
+		ret = ENOMEM;
+		goto out;
+	}
+
+	txt_file = fopen(txt_filename, "r");
+	if (!txt_file)
+		goto out;
+	/* Read the raw configuration file and transform it */
+	dataptr = calloc(datasize, 1);
+	if (!dataptr)
+		goto out;
+
+	envsize = datasize - (CRC_SIZE + redundant);
+	envptr = dataptr + CRC_SIZE + redundant;
+
+	ret = make_binary_config(txt_file, envptr, envsize);
+	ret = fclose(txt_file);
+
+	crc = crc32(0, envptr, envsize);
+	printf("crc: 0x%08X\n", crc);
+
+	*((uint32_t*) dataptr) = bigendian ? htobe32(crc) : htole32(crc);
+
+	bin_file = fopen(bin_filename, "w");
+	if (fwrite(dataptr, 1, datasize, bin_file) != datasize)
+		fprintf(stderr, "fwrite() failed: %s\n", strerror(errno));
+
+	ret = fclose(bin_file);
+
+out:
+	if (txt_filename)
+		free(txt_filename);
+	if (bin_filename)
+		free(bin_filename);
+	return ret;
+}
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-05 14:49 [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file David Wagner
@ 2011-08-05 15:23 ` Thomas Petazzoni
  2011-08-05 16:23 ` [U-Boot] [PATCHv2] " David Wagner
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 31+ messages in thread
From: Thomas Petazzoni @ 2011-08-05 15:23 UTC (permalink / raw)
  To: u-boot

Hello,

Le Fri,  5 Aug 2011 16:49:58 +0200,
David Wagner <david.wagner@free-electrons.com> a ?crit :

> This tool takes a key=value configuration file (same as would a `printenv' show)
> and generates the corresponding environnment image, ready to be flashed.

Nice tool. I'm currently using a crappy shell-based equivalent of this,
but it'd be a lot better to have a clean and nice version integrated in
U-Boot. However, see some comments below:

> +	       "\t-r : the environnment is redundand\n"

"the environment has two copies in flash" might be clearer that this
"redundand" word maybe.

> +static int make_binary_config(FILE* txt_file, unsigned char *envptr, int envsize)
> +{
> +	int i;
> +	int ret;
> +
> +	ret = fread(envptr, envsize, 1, txt_file);
> +	for (i = 0 ; i < envsize ; i++)
> +		if (envptr[i] == '\n')
> +			envptr[i] = '\0';
> +
> +	return 0;
> +}

The name of the function sounds a bit strange, and the function's job
is really small. Maybe it should just be merged into the main function.

> +int main(int argc, char **argv)
> +{
> +	uint32_t crc;
> +	char *txt_filename = NULL, *bin_filename = NULL;
> +	FILE *txt_file, *bin_file;
> +	unsigned char *dataptr, *envptr;
> +	unsigned int envsize, datasize = 0;
> +	int bigendian = 0;
> +	int redundant = 0;
> +
> +	int option;
> +	int ret = EXIT_SUCCESS;
> +
> +	opterr = 0;
> +
> +
> +	/* Parse the cmdline */
> +	while ((option = getopt(argc, argv, "s:o:rbh")) != -1)

I'd prefer to have an opening brace here, even if there is technically
a single statement inside the while() loop.

> +		switch (option)
> +		{
> +		case 's':
> +			datasize = atoi(optarg);
> +			break;

It'd be nicer if sizes could be given in decimal *or* hexadecimal
formats. 0x20000 is much easier to type than 131072.

> +		default:
> +			if (bin_filename)
> +				free(bin_filename);

Don't try to do needless cleanup, and let the operating system do it
for you. It's a short-lived program, I don't think it's worth worrying
about cleaning-up things.

> +			fprintf(stderr, "Wrong option\n");
> +			usage();
> +			return EXIT_FAILURE;
> +		}
> +
> +	if (datasize == 0) {
> +		printf("Please specify the size of the envrionnment partition.\n");

                                                       ^^^^^ typo
The message should probably be printed to stderr:

		fprintf(stderr, "blabla\n");

> +		usage();
> +		ret = EXIT_FAILURE;
> +		goto out;

just return EXIT_FAILURE, not need to clean up.

> +	}
> +	
> +
> +	txt_filename = strdup(argv[optind]);
> +	if (!txt_filename) {
> +		ret = ENOMEM;
> +		goto out;

no need to clean up.

> +	}
> +
> +	txt_file = fopen(txt_filename, "r");
> +	if (!txt_file)
> +		goto out;

You should check whether the size of the environment given in the file
doesn't exceed the size of the environment passed on the command line.

> +	/* Read the raw configuration file and transform it */
> +	dataptr = calloc(datasize, 1);
> +	if (!dataptr)
> +		goto out;

No need to clean up.

> +	envsize = datasize - (CRC_SIZE + redundant);
> +	envptr = dataptr + CRC_SIZE + redundant;
> +
> +	ret = make_binary_config(txt_file, envptr, envsize);
> +	ret = fclose(txt_file);
> +
> +	crc = crc32(0, envptr, envsize);
> +	printf("crc: 0x%08X\n", crc);

I don't think printing the CRC is useful, just drop.

> +	*((uint32_t*) dataptr) = bigendian ? htobe32(crc) : htole32(crc);

I think it'd be better to have :

struct env_normal {
	uint32_t	crc;
	char		data[0];
}

struct env_redund {
	uint32_t	crc;
	char		flags;
	char		data[0];
}

rather than this cast.

> +	bin_file = fopen(bin_filename, "w");
> +	if (fwrite(dataptr, 1, datasize, bin_file) != datasize)
> +		fprintf(stderr, "fwrite() failed: %s\n", strerror(errno));

Missing exit with error here.

> +	ret = fclose(bin_file);
> +
> +out:
> +	if (txt_filename)
> +		free(txt_filename);
> +	if (bin_filename)
> +		free(bin_filename);
> +	return ret;

No need to do useless clean up.

Regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv2] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-05 14:49 [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file David Wagner
  2011-08-05 15:23 ` Thomas Petazzoni
@ 2011-08-05 16:23 ` David Wagner
  2011-08-06 11:18   ` Mike Frysinger
  2011-08-09 10:31 ` [U-Boot] [PATCH] " David Wagner
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 31+ messages in thread
From: David Wagner @ 2011-08-05 16:23 UTC (permalink / raw)
  To: u-boot

This tool takes a key=value configuration file (same as would a `printenv' show)
and generates the corresponding environnment image, ready to be flashed.

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---

	Hi Thomas,

This second version addresses all your comments except - as we discussed - the
cast vs. struct part.

 tools/Makefile     |    5 ++
 tools/mkenvimage.c |  187 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 192 insertions(+), 0 deletions(-)
 create mode 100644 tools/mkenvimage.c

diff --git a/tools/Makefile b/tools/Makefile
index e813e1d..db8522f 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -69,6 +69,7 @@ BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX)
 BIN_FILES-y += mkimage$(SFX)
 BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX)
 BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
+BIN_FILES-y += mkenvimage$(SFX)
 
 # Source files which exist outside the tools directory
 EXT_OBJ_FILES-$(CONFIG_BUILD_ENVCRC) += common/env_embedded.o
@@ -93,6 +94,7 @@ OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o
 NOPED_OBJ_FILES-y += os_support.o
 OBJ_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1.o
 NOPED_OBJ_FILES-y += ublimage.o
+NOPED_OBJ_FILES-y += mkenvimage.o
 
 # Don't build by default
 #ifeq ($(ARCH),ppc)
@@ -171,6 +173,9 @@ $(obj)bmp_logo$(SFX):	$(obj)bmp_logo.o
 $(obj)envcrc$(SFX):	$(obj)crc32.o $(obj)env_embedded.o $(obj)envcrc.o $(obj)sha1.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
 
+$(obj)mkenvimage$(SFX):	$(obj)crc32.o $(obj)mkenvimage.o
+	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
+
 $(obj)gen_eth_addr$(SFX):	$(obj)gen_eth_addr.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
 	$(HOSTSTRIP) $@
diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
new file mode 100644
index 0000000..175126a
--- /dev/null
+++ b/tools/mkenvimage.c
@@ -0,0 +1,187 @@
+/*
+ * (C) Copyright 2011 Free Electrons
+ * David Wagner <david.wagner@free-electrons.com>
+ * 
+ * Inspired from envcrc.c:
+ * (C) Copyright 2001
+ * Paolo Scaffardi, AIRVENT SAM s.p.a - RIMINI(ITALY), arsenio at tin.it
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <errno.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <endian.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+extern uint32_t crc32 (uint32_t, const unsigned char *, unsigned int);
+
+#define CRC_SIZE sizeof(uint32_t)
+
+static void usage(void)
+{
+	printf("envcrc [-h] [-r] [-b] -s <envrionnment partition size> -o <output> "
+	       "<input file>\n"
+	       "\n"
+	       "\tThe input file is in format:\n"
+	       "\t\tkey1=value1\n"
+	       "\t\tkey2=value2\n"
+	       "\t\t...\n"
+	       "\t-r : the environment has two copies in flash\n"
+	       "\t-b : the target is big endian (default is little endian)\n");
+
+}
+
+int main(int argc, char **argv)
+{
+	uint32_t crc;
+	char *txt_filename = NULL, *bin_filename = NULL;
+	FILE *txt_file, *bin_file;
+	unsigned char *dataptr, *envptr;
+	unsigned int envsize, datasize = 0;
+	int bigendian = 0;
+	int redundant = 0;
+
+	int option;
+	int ret = EXIT_SUCCESS;
+
+	struct stat txt_file_stat;
+
+	int i;
+
+	opterr = 0;
+
+
+	/* Parse the cmdline */
+	while ((option = getopt(argc, argv, "s:o:rbh")) != -1) {
+		switch (option)
+		{
+		case 's':
+			datasize = strtol(optarg, NULL, 0);
+			break;
+		case 'o':
+			bin_filename = strdup(optarg);
+			if (!bin_filename) {
+				fprintf(stderr, "Can't strdup() the output filename\n");
+				return EXIT_FAILURE;
+			}
+			break;
+		case 'r':
+			redundant = 1;
+			break;
+		case 'b':
+			bigendian = 1;
+			break;
+		case 'h':
+			usage();
+			return EXIT_SUCCESS;
+		default:
+			fprintf(stderr, "Wrong option -%c\n", option);
+			usage();
+			return EXIT_FAILURE;
+		}
+	}
+
+
+	/* Check datasize and allocate the data */
+	if (datasize == 0) {
+		fprintf(stderr,
+			"Please specify the size of the envrionnment partition.\n");
+		usage();
+		return EXIT_FAILURE;
+	}
+
+	dataptr = calloc(datasize, 1);
+	if (!dataptr) {
+		fprintf(stderr, "Can't alloc dataptr.\n");
+		return EXIT_FAILURE;
+	}
+
+	/* envptr points to the beginning of the actual environment (after the
+	 * crc and possible `redundant' bit */
+	envsize = datasize - (CRC_SIZE + redundant);
+	envptr = dataptr + CRC_SIZE + redundant;
+
+
+	/* Check the configuration file ...*/
+	txt_filename = strdup(argv[optind]);
+	if (!txt_filename) {
+		fprintf(stderr, "Can't strdup() the configuration filename\n");
+		return EXIT_FAILURE;
+	}
+	ret = stat(txt_filename, &txt_file_stat);
+	if (ret == -1) {
+		fprintf(stderr, "Can't stat() on configuration file: %s",
+				strerror(errno));
+		return EXIT_FAILURE;
+	}
+	if (txt_file_stat.st_size > envsize) {
+		fprintf(stderr, "The configuration file is larger than the "
+				"envrionnment partition size\n");
+		return EXIT_FAILURE;
+	}
+
+	/* ... and open it. */
+	txt_file = fopen(txt_filename, "r");
+	if (!txt_file) {
+		fprintf(stderr, "Can't open configuration file: %s", strerror(errno));
+		return EXIT_FAILURE;
+	}
+		
+
+	/* Read the raw configuration file and transform it */
+	ret = fread(envptr, txt_file_stat.st_size, 1, txt_file);
+	if (ret != 1) {
+		fprintf(stderr, "Can't read the whole configuration file\n");
+		return EXIT_FAILURE;
+	}
+
+	for (i = 0 ; i < envsize ; i++)
+		if (envptr[i] == '\n')
+			envptr[i] = '\0';
+
+	ret = fclose(txt_file);
+
+
+	/* Computes the CRC and put it@the beginning of the data */
+	crc = crc32(0, envptr, envsize);
+
+	*((uint32_t*) dataptr) = bigendian ? htobe32(crc) : htole32(crc);
+
+	bin_file = fopen(bin_filename, "w");
+	if (!bin_file) {
+		fprintf(stderr, "Can't open output file: %s", strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	if (fwrite(dataptr, 1, datasize, bin_file) != datasize) {
+		fprintf(stderr, "fwrite() failed: %s\n", strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	ret = fclose(bin_file);
+
+
+	return ret;
+}
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv2] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-05 16:23 ` [U-Boot] [PATCHv2] " David Wagner
@ 2011-08-06 11:18   ` Mike Frysinger
  2011-08-08  8:16     ` David Wagner
  0 siblings, 1 reply; 31+ messages in thread
From: Mike Frysinger @ 2011-08-06 11:18 UTC (permalink / raw)
  To: u-boot

On Fri, Aug 5, 2011 at 09:23, David Wagner wrote:
> +#include <endian.h>

this is not portable.  include compiler.h and use the already existing
uswap/cpu_to/to_cpu/etc... helpers.

> +static void usage(void)
> +{
> + ? ? ? printf("envcrc [-h] [-r] [-b] -s <envrionnment partition size> -o <output> "

funny, "envcrc" isnt what the filename actually is ...

typo with "environment"

seems like this should also take a padding byte so people can use a
more sensible 0xff rather than 0x00

> + ? ? ? ? ? ? ?"\t-b : the target is big endian (default is little endian)\n");
> +
> +}

kill useless newline before brace

> + ? ? ? opterr = 0;

unnecessary ... punt it

> + ? ? ? ? ? ? ? switch (option)
> + ? ? ? ? ? ? ? {

cuddle the brace up

> + ? ? ? /* Check the configuration file ...*/
> + ? ? ? txt_filename = strdup(argv[optind]);

no need to strdup this.  argv isnt going anywhere.

> + ? ? ? ret = stat(txt_filename, &txt_file_stat);
> + ? ? ? if (ret == -1) {
> + ? ? ? ? ? ? ? fprintf(stderr, "Can't stat() on configuration file: %s",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? strerror(errno));
> + ? ? ? ? ? ? ? return EXIT_FAILURE;
> + ? ? ? }
> + ? ? ? if (txt_file_stat.st_size > envsize) {
> + ? ? ? ? ? ? ? fprintf(stderr, "The configuration file is larger than the "
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "envrionnment partition size\n");
> + ? ? ? ? ? ? ? return EXIT_FAILURE;
> + ? ? ? }
> +
> + ? ? ? /* ... and open it. */
> + ? ? ? txt_file = fopen(txt_filename, "r");

fopen() it, then fstat() the fileno(txt_file) to avoid possible race
conditions.  also, use "rb".

> + ? ? ? /* Read the raw configuration file and transform it */
> + ? ? ? ret = fread(envptr, txt_file_stat.st_size, 1, txt_file);

you've swapped size and nemb args

> + ? ? ? for (i = 0 ; i < envsize ; i++)
> + ? ? ? ? ? ? ? if (envptr[i] == '\n')
> + ? ? ? ? ? ? ? ? ? ? ? envptr[i] = '\0';

you could use memchr() and a while loop instead ... but probably not
worth the effort

> + ? ? ? ret = fclose(txt_file);

you could fclose() before the envptr conversion since you're done with
the file at that point

> + ? ? ? *((uint32_t*) dataptr) = bigendian ? htobe32(crc) : htole32(crc);

create a local uint32_t variable to set, then use memcpy to copy it
over to dataptr

> + ? ? ? bin_file = fopen(bin_filename, "w");

"wb"

> + ? ? ? if (fwrite(dataptr, 1, datasize, bin_file) != datasize) {

funny enough, you got the size/nemb args in the right order here ;)
-mike

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv2] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-06 11:18   ` Mike Frysinger
@ 2011-08-08  8:16     ` David Wagner
  2011-08-08  8:46       ` Albert ARIBAUD
  2011-08-08 18:53       ` Mike Frysinger
  0 siblings, 2 replies; 31+ messages in thread
From: David Wagner @ 2011-08-08  8:16 UTC (permalink / raw)
  To: u-boot

	Hi,

On 08/06/2011 01:18 PM, Mike Frysinger wrote:
> On Fri, Aug 5, 2011 at 09:23, David Wagner wrote:
>> +#include <endian.h>
>
> this is not portable.  include compiler.h and use the already existing
> uswap/cpu_to/to_cpu/etc... helpers.
>
>> +static void usage(void)
>> +{
>> +       printf("envcrc [-h] [-r] [-b] -s <envrionnment partition
size> -o <output> "
>
> funny, "envcrc" isnt what the filename actually is ...

ah, yes, it began as an attempt to extend envcrc but ended up fairly
different.

> seems like this should also take a padding byte so people can use a
> more sensible 0xff rather than 0x00

Thomas told me that when padding with 0xff, his environment image
wouldn't be taken into account anymore.
I'll add the option anyway, but do you know what could be happening ?

Also, I guess this makes necessary making sure there is a \0 after the
last configuration line.

>> +       if (fwrite(dataptr, 1, datasize, bin_file) != datasize) {
>
> funny enough, you got the size/nemb args in the right order here ;)
> -mike

I wasn't sure whether the proper way of doing it was "read/write 1 byte,
N times" or "read/write the size of the file, 1 time".  I probably
changed my mine in between.


	Thanks for reviewing ;
	David.

-- 
David Wagner, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv2] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-08  8:16     ` David Wagner
@ 2011-08-08  8:46       ` Albert ARIBAUD
  2011-08-08  8:56         ` David Wagner
  2011-08-08 18:53       ` Mike Frysinger
  1 sibling, 1 reply; 31+ messages in thread
From: Albert ARIBAUD @ 2011-08-08  8:46 UTC (permalink / raw)
  To: u-boot

Hi David,

Le 08/08/2011 10:16, David Wagner a ?crit :

>>> +static void usage(void)
>>> +{
>>> +       printf("envcrc [-h] [-r] [-b] -s<envrionnment partition
> size>  -o<output>  "
>>
>> funny, "envcrc" isnt what the filename actually is ...
>
> ah, yes, it began as an attempt to extend envcrc but ended up fairly
> different.

Maybe passing argv[0] to usage() would help deal with the current name 
of the executable?

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv2] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-08  8:46       ` Albert ARIBAUD
@ 2011-08-08  8:56         ` David Wagner
  0 siblings, 0 replies; 31+ messages in thread
From: David Wagner @ 2011-08-08  8:56 UTC (permalink / raw)
  To: u-boot

On 08/08/2011 10:46 AM, Albert ARIBAUD wrote:
> Hi David,
> 
> Le 08/08/2011 10:16, David Wagner a ?crit :
> 
>>>> +static void usage(void)
>>>> +{
>>>> +       printf("envcrc [-h] [-r] [-b] -s<envrionnment partition
>> size>  -o<output>  "
>>>
>>> funny, "envcrc" isnt what the filename actually is ...
>>
>> ah, yes, it began as an attempt to extend envcrc but ended up fairly
>> different.
> 
> Maybe passing argv[0] to usage() would help deal with the current name
> of the executable?
> 
> Amicalement,

Indeed, merci.

-- 
David Wagner, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv2] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-08  8:16     ` David Wagner
  2011-08-08  8:46       ` Albert ARIBAUD
@ 2011-08-08 18:53       ` Mike Frysinger
  1 sibling, 0 replies; 31+ messages in thread
From: Mike Frysinger @ 2011-08-08 18:53 UTC (permalink / raw)
  To: u-boot

On Mon, Aug 8, 2011 at 04:16, David Wagner wrote:
> On 08/06/2011 01:18 PM, Mike Frysinger wrote:
>> seems like this should also take a padding byte so people can use a
>> more sensible 0xff rather than 0x00
>
> Thomas told me that when padding with 0xff, his environment image
> wouldn't be taken into account anymore.
> I'll add the option anyway, but do you know what could be happening ?

sorry, but i dont know what you're referring to.  perhaps you wrote
out the padding before calculating the crc ?  ive been using
`tools/envcrc --binary` for a while now and it pads things with 0xff.

> Also, I guess this makes necessary making sure there is a \0 after the
> last configuration line.

two to be exact ... one for the config opt, and one to tell u-boot
that it's the end of the env.

> I wasn't sure whether the proper way of doing it was "read/write 1 byte,
> N times" or "read/write the size of the file, 1 time". ?I probably
> changed my mine in between.

dont think of it in terms of bytes.  in fact, forget about that
completely.  think of it in terms of "elements".  in this case, the
element is a "char" which means you want to read a bunch of "char"
elements and the size of each is 1.

you could also do:
fwrite(dataptr, sizeof(*dataptr), datasize, bin_file);
then you dont need to think "how many bytes is one element".
-mike

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-05 14:49 [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file David Wagner
  2011-08-05 15:23 ` Thomas Petazzoni
  2011-08-05 16:23 ` [U-Boot] [PATCHv2] " David Wagner
@ 2011-08-09 10:31 ` David Wagner
  2011-08-22  1:09   ` Mike Frysinger
  2011-08-24 22:12   ` Wolfgang Denk
  2011-08-29 12:06 ` [U-Boot] [PATCHv4] " David Wagner
                   ` (7 subsequent siblings)
  10 siblings, 2 replies; 31+ messages in thread
From: David Wagner @ 2011-08-09 10:31 UTC (permalink / raw)
  To: u-boot

This tool takes a key=value configuration file (same as would a `printenv' show)
and generates the corresponding environnment image, ready to be flashed.

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---

	Hi Mike,

This 3rd version should address what you pointed out.

I had troubles testing it on my IGEP board: I couldn't compile from the official
git (I think it's because of my toolchain ... a more recent toolchain might be
able to compile but won't compile IGEP's uboot version ... weird stuff above my
head involved).  And the IGEP uboot seems broken wrt environnment in onenand:
when computing the CRC, it only takes the 4092 (4096 minus the header) bytes
of the environment into account ...

So, someone else should probably test it.

	David.

 tools/Makefile     |    5 ++
 tools/mkenvimage.c |  205 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 210 insertions(+), 0 deletions(-)
 create mode 100644 tools/mkenvimage.c

diff --git a/tools/Makefile b/tools/Makefile
index e813e1d..db8522f 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -69,6 +69,7 @@ BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX)
 BIN_FILES-y += mkimage$(SFX)
 BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX)
 BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
+BIN_FILES-y += mkenvimage$(SFX)
 
 # Source files which exist outside the tools directory
 EXT_OBJ_FILES-$(CONFIG_BUILD_ENVCRC) += common/env_embedded.o
@@ -93,6 +94,7 @@ OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o
 NOPED_OBJ_FILES-y += os_support.o
 OBJ_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1.o
 NOPED_OBJ_FILES-y += ublimage.o
+NOPED_OBJ_FILES-y += mkenvimage.o
 
 # Don't build by default
 #ifeq ($(ARCH),ppc)
@@ -171,6 +173,9 @@ $(obj)bmp_logo$(SFX):	$(obj)bmp_logo.o
 $(obj)envcrc$(SFX):	$(obj)crc32.o $(obj)env_embedded.o $(obj)envcrc.o $(obj)sha1.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
 
+$(obj)mkenvimage$(SFX):	$(obj)crc32.o $(obj)mkenvimage.o
+	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
+
 $(obj)gen_eth_addr$(SFX):	$(obj)gen_eth_addr.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
 	$(HOSTSTRIP) $@
diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
new file mode 100644
index 0000000..fe91bc4
--- /dev/null
+++ b/tools/mkenvimage.c
@@ -0,0 +1,205 @@
+/*
+ * (C) Copyright 2011 Free Electrons
+ * David Wagner <david.wagner@free-electrons.com>
+ * 
+ * Inspired from envcrc.c:
+ * (C) Copyright 2001
+ * Paolo Scaffardi, AIRVENT SAM s.p.a - RIMINI(ITALY), arsenio at tin.it
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <errno.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <compiler.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+extern uint32_t crc32 (uint32_t, const unsigned char *, unsigned int);
+
+#define CRC_SIZE sizeof(uint32_t)
+
+static void usage(char *exec_name)
+{
+	printf("%s [-h] [-r] [-b] [-p <byte>] "
+	       "-s <envrionment partition size> -o <output> <input file>\n"
+	       "\n"
+	       "\tThe input file is in format:\n"
+	       "\t\tkey1=value1\n"
+	       "\t\tkey2=value2\n"
+	       "\t\t...\n"
+	       "\t-r : the environment has two copies in flash\n"
+	       "\t-b : the target is big endian (default is little endian)\n"
+	       "\t-p <byte> : fill the image with <byte> bytes instead of 0xff bytes\n",
+	       exec_name);
+}
+
+int main(int argc, char **argv)
+{
+	uint32_t crc, targetendian_crc;
+	char *txt_filename = NULL, *bin_filename = NULL;
+	FILE *txt_file, *bin_file;
+	unsigned char *dataptr, *envptr;
+	unsigned int envsize, datasize = 0;
+	int bigendian = 0;
+	int redundant = 0;
+	unsigned char padbyte = 0xff;
+
+	int option;
+	int ret = EXIT_SUCCESS;
+
+	struct stat txt_file_stat;
+
+	int i;
+
+
+	/* Parse the cmdline */
+	while ((option = getopt(argc, argv, "s:o:rbp:h")) != -1) {
+		switch (option) {
+		case 's':
+			datasize = strtol(optarg, NULL, 0);
+			break;
+		case 'o':
+			bin_filename = strdup(optarg);
+			if (!bin_filename) {
+				fprintf(stderr, "Can't strdup() the output filename\n");
+				return EXIT_FAILURE;
+			}
+			break;
+		case 'r':
+			redundant = 1;
+			break;
+		case 'b':
+			bigendian = 1;
+			break;
+		case 'p':
+			padbyte = strtol(optarg, NULL, 0);
+			break;
+		case 'h':
+			usage(argv[0]);
+			return EXIT_SUCCESS;
+		default:
+			fprintf(stderr, "Wrong option -%c\n", option);
+			usage(argv[0]);
+			return EXIT_FAILURE;
+		}
+	}
+
+
+	/* Check datasize and allocate the data */
+	if (datasize == 0) {
+		fprintf(stderr,
+			"Please specify the size of the envrionnment partition.\n");
+		usage(argv[0]);
+		return EXIT_FAILURE;
+	}
+
+	dataptr = calloc(datasize, sizeof(*dataptr));
+	if (!dataptr) {
+		fprintf(stderr, "Can't alloc dataptr.\n");
+		return EXIT_FAILURE;
+	}
+
+	/* envptr points to the beginning of the actual environment (after the
+	 * crc and possible `redundant' bit */
+	envsize = datasize - (CRC_SIZE + redundant);
+	envptr = dataptr + CRC_SIZE + redundant;
+
+	/* Pad the environnment with the padding byte */
+	memset(envptr, padbyte, envsize);
+
+	/* Open the configuration file ... */
+	txt_filename = argv[optind];
+	if (!txt_filename) {
+		fprintf(stderr, "Can't strdup() the configuration filename\n");
+		return EXIT_FAILURE;
+	}
+	txt_file = fopen(txt_filename, "rb");
+	if (!txt_file) {
+		fprintf(stderr, "Can't open configuration file: %s\n", strerror(errno));
+		return EXIT_FAILURE;
+	}
+	/* ... and check it */
+	ret = fstat(fileno(txt_file), &txt_file_stat);
+	if (ret == -1) {
+		fprintf(stderr, "Can't stat() on configuration file: %s\n",
+				strerror(errno));
+		return EXIT_FAILURE;
+	}
+	if (txt_file_stat.st_size > envsize) {
+		fprintf(stderr, "The configuration file is larger than the "
+				"envrionnment partition size\n");
+		return EXIT_FAILURE;
+	}
+		
+
+	/* Read the raw configuration file and transform it */
+	ret = fread(envptr, sizeof(*envptr), txt_file_stat.st_size, txt_file);
+	if (ret != txt_file_stat.st_size) {
+		fprintf(stderr, "Can't read the whole configuration file\n");
+		return EXIT_FAILURE;
+	}
+	ret = fclose(txt_file);
+
+	for (i = 0 ; i < txt_file_stat.st_size ; i++)
+		if (envptr[i] == '\n')
+			envptr[i] = '\0';
+	/* 
+	 * Make sure there is a final '\0' (necessary if the padding byte isn't
+	 * 0x00 or if there wasn't a newline at the end of the configuration
+	 * file) Also, don't add it if the configuration file size is exactly
+	 * the size of the environnment.
+	 *
+	 * And do it again on the next byte to mark the end of the environment.
+	 */
+	if (i < envsize && envptr[i-1] != '\0') {
+		envptr[i] = '\0';
+		i++;
+		envptr[i] = '\0';
+	} else if (i < envsize) {
+		envptr[i] = '\0';
+	}
+
+
+	/* Computes the CRC and put it@the beginning of the data */
+	crc = crc32(0, envptr, envsize);
+	targetendian_crc = bigendian ? cpu_to_be32(crc) : cpu_to_le32(crc);
+
+	memcpy((uint32_t*) dataptr, &targetendian_crc, sizeof(uint32_t));
+
+	bin_file = fopen(bin_filename, "wb");
+	if (!bin_file) {
+		fprintf(stderr, "Can't open output file: %s\n", strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	if (fwrite(dataptr, sizeof(*dataptr), datasize, bin_file) != datasize) {
+		fprintf(stderr, "fwrite() failed: %s\n", strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	ret = fclose(bin_file);
+
+
+	return ret;
+}
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-09 10:31 ` [U-Boot] [PATCH] " David Wagner
@ 2011-08-22  1:09   ` Mike Frysinger
  2011-08-24 22:12   ` Wolfgang Denk
  1 sibling, 0 replies; 31+ messages in thread
From: Mike Frysinger @ 2011-08-22  1:09 UTC (permalink / raw)
  To: u-boot

On Tuesday, August 09, 2011 06:31:29 David Wagner wrote:
> +extern uint32_t crc32 (uint32_t, const unsigned char *, unsigned int);

does including u-boot/crc.h work ?

> +static void usage(char *exec_name)

const char *exec_name

> +	char *txt_filename = NULL, *bin_filename = NULL;

mark them both const ?

> +	dataptr = calloc(datasize, sizeof(*dataptr));

you zero-ed it out here ...

> +	/* Pad the environnment with the padding byte */
> +	memset(envptr, padbyte, envsize);

... then manually filled it with a byte.  just use malloc() instead of 
calloc() and avoid that zero.

> +	memcpy((uint32_t*) dataptr, &targetendian_crc, sizeof(uint32_t));

i dont think that cast is necessary.  just punt it.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110821/d79e0d6e/attachment.pgp 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-09 10:31 ` [U-Boot] [PATCH] " David Wagner
  2011-08-22  1:09   ` Mike Frysinger
@ 2011-08-24 22:12   ` Wolfgang Denk
  1 sibling, 0 replies; 31+ messages in thread
From: Wolfgang Denk @ 2011-08-24 22:12 UTC (permalink / raw)
  To: u-boot

Dear David Wagner,

In message <1312885889-20222-1-git-send-email-david.wagner@free-electrons.com> you wrote:
> This tool takes a key=value configuration file (same as would a `printenv' show)
> and generates the corresponding environnment image, ready to be flashed.

s/nnm/nm/

> 
> Signed-off-by: David Wagner <david.wagner@free-electrons.com>
> ---
> 
> 	Hi Mike,
> 
> This 3rd version should address what you pointed out.

If this is v3, then why don't you say so in the Subject: ???


Could you please explain which usage scenarios you have in mind for
this tool?  I would probably rather just load the text file you use as
input, and run "env import -t" on it.

Checkpatch says:

total: 4 errors, 5 warnings, 228 lines checked

Please fix these.

In addition:

...
> +		default:
> +			fprintf(stderr, "Wrong option -%c\n", option);
> +			usage(argv[0]);
> +			return EXIT_FAILURE;
> +		}
> +	}
> +
> +
> +	/* Check datasize and allocate the data */

Please only one blank line in cases like this.

> +	/* envptr points to the beginning of the actual environment (after the
> +	 * crc and possible `redundant' bit */

inforrect multiline comment style.

> +	/* Open the configuration file ... */
> +	txt_filename = argv[optind];
> +	if (!txt_filename) {
> +		fprintf(stderr, "Can't strdup() the configuration filename\n");
> +		return EXIT_FAILURE;

Where is there any strdup() involved ?

> +	txt_file = fopen(txt_filename, "rb");
> +	if (!txt_file) {
> +		fprintf(stderr, "Can't open configuration file: %s\n", strerror(errno));

It would probably be very useful to also print the file name.

> +	/* ... and check it */
> +	ret = fstat(fileno(txt_file), &txt_file_stat);

Why don't you simply use mmap() ?

> +	for (i = 0 ; i < txt_file_stat.st_size ; i++)
> +		if (envptr[i] == '\n')
> +			envptr[i] = '\0';

This is actually wrong.  Envrionment variables can have embedded
newlines.

> +	/* 
> +	 * Make sure there is a final '\0' (necessary if the padding byte isn't
> +	 * 0x00 or if there wasn't a newline at the end of the configuration

You did not take this into account in your file size check above, it
seems.

> +	 * file) Also, don't add it if the configuration file size is exactly
> +	 * the size of the environnment.

The double '\0' termination is _always_ needed.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You can observe a lot just by watchin'.                  - Yogi Berra

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv4] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-05 14:49 [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file David Wagner
                   ` (2 preceding siblings ...)
  2011-08-09 10:31 ` [U-Boot] [PATCH] " David Wagner
@ 2011-08-29 12:06 ` David Wagner
  2011-08-30 19:12   ` Mike Frysinger
  2011-08-30 19:34   ` Wolfgang Denk
  2011-09-01 15:57 ` [U-Boot] [PATCHv5] " David Wagner
                   ` (6 subsequent siblings)
  10 siblings, 2 replies; 31+ messages in thread
From: David Wagner @ 2011-08-29 12:06 UTC (permalink / raw)
  To: u-boot

This tool takes a key=value configuration file (same as would a `printenv' show)
and generates the corresponding environment image, ready to be flashed.

For now, it doesn't work properly if environment variables have embedded
newlines.

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---

	changes since v3
	~~~~~~~~~~~~~~~~

 * cosmetic changes (checkpatch, spelling, blank lines)

 * fail with an error message if it's impossible to have two ending '\0'

 @Wolgang:
What is the advantage of using mmap() here ? the file is read entirely and
sequentially ; does it make much of a difference compared to fread() ?

PS: Until a proper way is found for replacing only newlines that separate two
environment variables (and not the ones inside a variable), let's just warn that
it isn't supported.

 tools/Makefile     |    5 +
 tools/mkenvimage.c |  219 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 224 insertions(+), 0 deletions(-)
 create mode 100644 tools/mkenvimage.c

diff --git a/tools/Makefile b/tools/Makefile
index fc741d3..8e7e85e 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -69,6 +69,7 @@ BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX)
 BIN_FILES-y += mkimage$(SFX)
 BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX)
 BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
+BIN_FILES-y += mkenvimage$(SFX)
 
 # Source files which exist outside the tools directory
 EXT_OBJ_FILES-$(CONFIG_BUILD_ENVCRC) += common/env_embedded.o
@@ -94,6 +95,7 @@ OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o
 NOPED_OBJ_FILES-y += os_support.o
 OBJ_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1.o
 NOPED_OBJ_FILES-y += ublimage.o
+NOPED_OBJ_FILES-y += mkenvimage.o
 
 # Don't build by default
 #ifeq ($(ARCH),ppc)
@@ -172,6 +174,9 @@ $(obj)bmp_logo$(SFX):	$(obj)bmp_logo.o
 $(obj)envcrc$(SFX):	$(obj)crc32.o $(obj)env_embedded.o $(obj)envcrc.o $(obj)sha1.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
 
+$(obj)mkenvimage$(SFX):	$(obj)crc32.o $(obj)mkenvimage.o
+	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
+
 $(obj)gen_eth_addr$(SFX):	$(obj)gen_eth_addr.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
 	$(HOSTSTRIP) $@
diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
new file mode 100644
index 0000000..9c06e1e
--- /dev/null
+++ b/tools/mkenvimage.c
@@ -0,0 +1,219 @@
+/*
+ * (C) Copyright 2011 Free Electrons
+ * David Wagner <david.wagner@free-electrons.com>
+ *
+ * Inspired from envcrc.c:
+ * (C) Copyright 2001
+ * Paolo Scaffardi, AIRVENT SAM s.p.a - RIMINI(ITALY), arsenio at tin.it
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <errno.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <compiler.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#include <u-boot/crc.h>
+
+#define CRC_SIZE sizeof(uint32_t)
+
+static void usage(const char *exec_name)
+{
+	printf("%s [-h] [-r] [-b] [-p <byte>] "
+	       "-s <envrionment partition size> -o <output> <input file>\n"
+	       "\n"
+	       "\tThe input file is in format:\n"
+	       "\t\tkey1=value1\n"
+	       "\t\tkey2=value2\n"
+	       "\t\t...\n"
+	       "\t-r : the environment has two copies in flash\n"
+	       "\t-b : the target is big endian (default is little endian)\n"
+	       "\t-p <byte> : fill the image with <byte> bytes instead of 0xff "
+	       "bytes\n",
+	       exec_name);
+}
+
+int main(int argc, char **argv)
+{
+	uint32_t crc, targetendian_crc;
+	const char *txt_filename = NULL, *bin_filename = NULL;
+	FILE *txt_file, *bin_file;
+	unsigned char *dataptr, *envptr;
+	unsigned int envsize, datasize = 0;
+	int bigendian = 0;
+	int redundant = 0;
+	unsigned char padbyte = 0xff;
+
+	int option;
+	int ret = EXIT_SUCCESS;
+
+	struct stat txt_file_stat;
+
+	int i;
+
+	/* Parse the cmdline */
+	while ((option = getopt(argc, argv, "s:o:rbp:h")) != -1) {
+		switch (option) {
+		case 's':
+			datasize = strtol(optarg, NULL, 0);
+			break;
+		case 'o':
+			bin_filename = strdup(optarg);
+			if (!bin_filename) {
+				fprintf(stderr, "Can't strdup() the output "
+						"filename\n");
+				return EXIT_FAILURE;
+			}
+			break;
+		case 'r':
+			redundant = 1;
+			break;
+		case 'b':
+			bigendian = 1;
+			break;
+		case 'p':
+			padbyte = strtol(optarg, NULL, 0);
+			break;
+		case 'h':
+			usage(argv[0]);
+			return EXIT_SUCCESS;
+		default:
+			fprintf(stderr, "Wrong option -%c\n", option);
+			usage(argv[0]);
+			return EXIT_FAILURE;
+		}
+	}
+
+	/* Check datasize and allocate the data */
+	if (datasize == 0) {
+		fprintf(stderr,
+			"Please specify the size of the envrionnment "
+			"partition.\n");
+		usage(argv[0]);
+		return EXIT_FAILURE;
+	}
+
+	dataptr = malloc(datasize * sizeof(*dataptr));
+	if (!dataptr) {
+		fprintf(stderr, "Can't alloc dataptr.\n");
+		return EXIT_FAILURE;
+	}
+
+	/*
+	 * envptr points to the beginning of the actual environment (after the
+	 * crc and possible `redundant' bit
+	 */
+	envsize = datasize - (CRC_SIZE + redundant);
+	envptr = dataptr + CRC_SIZE + redundant;
+
+	/* Pad the environnment with the padding byte */
+	memset(envptr, padbyte, envsize);
+
+	/* Open the configuration file ... */
+	if (optind >= argc) {
+		fprintf(stderr, "Please specify a configuration filename\n");
+		return EXIT_FAILURE;
+	}
+
+	txt_filename = argv[optind];
+	txt_file = fopen(txt_filename, "rb");
+	if (!txt_file) {
+		fprintf(stderr, "Can't open configuration file \"%s\": %s\n",
+				txt_filename, strerror(errno));
+		return EXIT_FAILURE;
+	}
+	/* ... and check it */
+	ret = fstat(fileno(txt_file), &txt_file_stat);
+	if (ret == -1) {
+		fprintf(stderr, "Can't stat() on configuration file \"%s\": "
+				" %s\n", txt_filename, strerror(errno));
+		return EXIT_FAILURE;
+	}
+	/*
+	 * The right test to do is "=>" (not ">") because of the additionnal
+	 * ending \0. See below.
+	 */
+	if (txt_file_stat.st_size >= envsize) {
+		fprintf(stderr, "The configuration file is larger than the "
+				"envrionnment partition size\n");
+		return EXIT_FAILURE;
+	}
+
+	/* Read the raw configuration file and transform it */
+	ret = fread(envptr, sizeof(*envptr), txt_file_stat.st_size, txt_file);
+	if (ret != txt_file_stat.st_size) {
+		fprintf(stderr, "Can't read the whole configuration file\n");
+		return EXIT_FAILURE;
+	}
+	ret = fclose(txt_file);
+
+	for (i = 0 ; i < txt_file_stat.st_size ; i++)
+		if (envptr[i] == '\n')
+			envptr[i] = '\0';
+	/*
+	 * Make sure there is a final '\0' (necessary if the padding byte isn't
+	 * 0x00 or if there wasn't a newline@the end of the configuration
+	 * file)
+	 *
+	 * And do it again on the next byte to mark the end of the environment.
+	 */
+	if (i < envsize && envptr[i-1] != '\0') {
+		envptr[i++] = '\0';
+		/*
+		 * The text file doesn't have an ending newline.  We need to
+		 * check the env size again to make sure we have room for two \0
+		 */
+		if (i >= envsize) {
+			fprintf(stderr, "The configuration is too large for the"
+					" target environment storage\n");
+			return EXIT_FAILURE;
+		}
+		envptr[i] = '\0';
+	} else if (i < envsize) {
+		envptr[i] = '\0';
+	}
+
+	/* Computes the CRC and put it@the beginning of the data */
+	crc = crc32(0, envptr, envsize);
+	targetendian_crc = bigendian ? cpu_to_be32(crc) : cpu_to_le32(crc);
+
+	memcpy(dataptr, &targetendian_crc, sizeof(uint32_t));
+
+	bin_file = fopen(bin_filename, "wb");
+	if (!bin_file) {
+		fprintf(stderr, "Can't open output file \"%s\": %s\n",
+				bin_filename, strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	if (fwrite(dataptr, sizeof(*dataptr), datasize, bin_file) != datasize) {
+		fprintf(stderr, "fwrite() failed: %s\n", strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	ret = fclose(bin_file);
+
+	return ret;
+}
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv4] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-29 12:06 ` [U-Boot] [PATCHv4] " David Wagner
@ 2011-08-30 19:12   ` Mike Frysinger
  2011-08-30 19:34   ` Wolfgang Denk
  1 sibling, 0 replies; 31+ messages in thread
From: Mike Frysinger @ 2011-08-30 19:12 UTC (permalink / raw)
  To: u-boot

On Monday, August 29, 2011 08:06:46 David Wagner wrote:
> This tool takes a key=value configuration file (same as would a `printenv'
> show) and generates the corresponding environment image, ready to be
> flashed.

this one line desc is pretty succulent.  i'd suggest adding it to the usage() 
so that people who run `./tools/mkenvimage -h` get the idea immediately.

the rest looks pretty straightforward without actually compiling/testing :).  
thanks !
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110830/e8e45e42/attachment.pgp 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv4] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-29 12:06 ` [U-Boot] [PATCHv4] " David Wagner
  2011-08-30 19:12   ` Mike Frysinger
@ 2011-08-30 19:34   ` Wolfgang Denk
  1 sibling, 0 replies; 31+ messages in thread
From: Wolfgang Denk @ 2011-08-30 19:34 UTC (permalink / raw)
  To: u-boot

Dear David Wagner,

In message <1314619606-1172-1-git-send-email-david.wagner@free-electrons.com> you wrote:
> This tool takes a key=value configuration file (same as would a `printenv' show)
> and generates the corresponding environment image, ready to be flashed.
> 
> For now, it doesn't work properly if environment variables have embedded
> newlines.

I think this should be added for compatibility with both the printenv
output and the result of "env export -t"

>  @Wolgang:
> What is the advantage of using mmap() here ? the file is read entirely and
> sequentially ; does it make much of a difference compared to fread() ?

It's easier to go back in the stream without allocating buffers that
are at least as big as the file.

> PS: Until a proper way is found for replacing only newlines that separate two
> environment variables (and not the ones inside a variable), let's just warn that
> it isn't supported.

What do you mean by "Until a proper way is found"?  There is nothing
to be found.  Just have a look at the "env import" code which does
exactly that.  Alternatively, as you are only dealing with text format
anyway, look if the character immediately preceeding the newline is a
backslash:

	=> setenv x 'line 1
	> line 2'
	=> printenv
	...
	x=line 1\
	line 2

> @@ -69,6 +69,7 @@ BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX)
>  BIN_FILES-y += mkimage$(SFX)
>  BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX)
>  BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
> +BIN_FILES-y += mkenvimage$(SFX)

Please keep list sorted.

>  # Source files which exist outside the tools directory
>  EXT_OBJ_FILES-$(CONFIG_BUILD_ENVCRC) += common/env_embedded.o
> @@ -94,6 +95,7 @@ OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o
>  NOPED_OBJ_FILES-y += os_support.o
>  OBJ_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1.o
>  NOPED_OBJ_FILES-y += ublimage.o
> +NOPED_OBJ_FILES-y += mkenvimage.o

Ditto.

>  # Don't build by default
>  #ifeq ($(ARCH),ppc)
> @@ -172,6 +174,9 @@ $(obj)bmp_logo$(SFX):	$(obj)bmp_logo.o
>  $(obj)envcrc$(SFX):	$(obj)crc32.o $(obj)env_embedded.o $(obj)envcrc.o $(obj)sha1.o
>  	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
>  
> +$(obj)mkenvimage$(SFX):	$(obj)crc32.o $(obj)mkenvimage.o
> +	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
> +
>  $(obj)gen_eth_addr$(SFX):	$(obj)gen_eth_addr.o
>  	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
>  	$(HOSTSTRIP) $@

Ditto.

...
> +static void usage(const char *exec_name)
> +{
> +	printf("%s [-h] [-r] [-b] [-p <byte>] "
> +	       "-s <envrionment partition size> -o <output> <input file>\n"
> +	       "\n"
> +	       "\tThe input file is in format:\n"
> +	       "\t\tkey1=value1\n"
> +	       "\t\tkey2=value2\n"
> +	       "\t\t...\n"
> +	       "\t-r : the environment has two copies in flash\n"

Please s/two/multiple/ or s/two/more than one/ - especially on NAND we
can have more than just 2 copies.

> +	if (datasize == 0) {
> +		fprintf(stderr,
> +			"Please specify the size of the envrionnment "

s/envrionnment/environment/

Please fix globally (same error further down below).

> +	/* Open the configuration file ... */
> +	if (optind >= argc) {
> +		fprintf(stderr, "Please specify a configuration filename\n");
> +		return EXIT_FAILURE;
> +	}

Why don;t you allow reading from stdin?  It is good old Unix tradition
that all commands can be used in pipes.  Also, input file name '-'
should select stdin.

Um... and why do you call it "configuration file"?  It's not
configuration data, it's plain input data, so rather call it "input
file"

> +	txt_filename = argv[optind];
> +	txt_file = fopen(txt_filename, "rb");
> +	if (!txt_file) {
> +		fprintf(stderr, "Can't open configuration file \"%s\": %s\n",
> +				txt_filename, strerror(errno));

Here it's even better to omit the "configuration file " part
completely.

> +	}
> +	/* ... and check it */
> +	ret = fstat(fileno(txt_file), &txt_file_stat);
> +	if (ret == -1) {
> +		fprintf(stderr, "Can't stat() on configuration file \"%s\": "
> +				" %s\n", txt_filename, strerror(errno));

Same here.

> +		return EXIT_FAILURE;
> +	}
> +	/*
> +	 * The right test to do is "=>" (not ">") because of the additionnal
> +	 * ending \0. See below.
> +	 */
> +	if (txt_file_stat.st_size >= envsize) {
> +		fprintf(stderr, "The configuration file is larger than the "
> +				"envrionnment partition size\n");

See note above.

> +	for (i = 0 ; i < txt_file_stat.st_size ; i++)
> +		if (envptr[i] == '\n')
> +			envptr[i] = '\0';

This needs braces.

> +	/*
> +	 * Make sure there is a final '\0' (necessary if the padding byte isn't
> +	 * 0x00 or if there wasn't a newline at the end of the configuration
> +	 * file)

The double '\0' termination is _always_ necessary.  Please avoid
constructing special cases where there aren't any.

> +	 * And do it again on the next byte to mark the end of the environment.
> +	 */
> +	if (i < envsize && envptr[i-1] != '\0') {
> +		envptr[i++] = '\0';
> +		/*
> +		 * The text file doesn't have an ending newline.  We need to
> +		 * check the env size again to make sure we have room for two \0
> +		 */
> +		if (i >= envsize) {
> +			fprintf(stderr, "The configuration is too large for the"
> +					" target environment storage\n");
> +			return EXIT_FAILURE;
> +		}

If you test this here, you can remove the test above.

> +	bin_file = fopen(bin_filename, "wb");
> +	if (!bin_file) {
> +		fprintf(stderr, "Can't open output file \"%s\": %s\n",
> +				bin_filename, strerror(errno));
> +		return EXIT_FAILURE;
> +	}
> +
> +	if (fwrite(dataptr, sizeof(*dataptr), datasize, bin_file) != datasize) {
> +		fprintf(stderr, "fwrite() failed: %s\n", strerror(errno));
> +		return EXIT_FAILURE;
> +	}
> +
> +	ret = fclose(bin_file);

Is there any good reason for using stdio functions (fopen(), fread(),
fwrite(), fclose()) instead of plain system calls (open(), read(),
write(), close()) ?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
What's the sound a name makes when it's dropped?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv5] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-05 14:49 [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file David Wagner
                   ` (3 preceding siblings ...)
  2011-08-29 12:06 ` [U-Boot] [PATCHv4] " David Wagner
@ 2011-09-01 15:57 ` David Wagner
  2011-09-01 17:47   ` Mike Frysinger
  2011-09-02  8:47 ` [U-Boot] [PATCH] " David Wagner
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 31+ messages in thread
From: David Wagner @ 2011-09-01 15:57 UTC (permalink / raw)
  To: u-boot

This tool takes a key=value configuration file (same as would a `printenv' show)
and generates the corresponding environment image, ready to be flashed.

use case: flash the environment with an external tool

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---

	changes since v4
	~~~~~~~~~~~~~~~~

 - keep the Makefiles somewhat sorted (I'm not sure how it is sorted but since
   "mkenvimage" is alphabetically before "mkimage", I put the former just before
   the latter).

 - replace f{open, read, write, ...} with {open, read, write, ...} and remove
   the now unused stdlib.h include

 - tweak usage()

 - typos

 - properly handle embedded newlines, comments and empty lines at the beginning
   of the input file.  This introduced a second buffer ('filebuf') (see the
   big for loop): When we encouter a newline after a backslash, without this
   buffer, we would have needed to shift all envptr one byte backward to
   overwrite the preceeding backslash.  Introducing this second buffer looked
   like a simpler alternative.

 - correct the way how 2 \0 are put at the end of the environment (@Wolfgang:
   the second "ep >= envsize" check is there to handle the corner case where the
   env target partition would be, say, 32 bytes long and the input file is 31
   byte long but doesn't have an ending newline)

PS: as the input file is probably always pretty small and we read the file
sequentially, we don't go back in the stream, we didn't find much advantage over
read()

 tools/Makefile     |    5 +
 tools/mkenvimage.c |  263 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 268 insertions(+), 0 deletions(-)
 create mode 100644 tools/mkenvimage.c

diff --git a/tools/Makefile b/tools/Makefile
index fc741d3..da7caf0 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -66,6 +66,7 @@ BIN_FILES-$(CONFIG_BUILD_ENVCRC) += envcrc$(SFX)
 BIN_FILES-$(CONFIG_CMD_NET) += gen_eth_addr$(SFX)
 BIN_FILES-$(CONFIG_CMD_LOADS) += img2srec$(SFX)
 BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX)
+BIN_FILES-y += mkenvimage$(SFX)
 BIN_FILES-y += mkimage$(SFX)
 BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX)
 BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
@@ -89,6 +90,7 @@ OBJ_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes.o
 NOPED_OBJ_FILES-y += kwbimage.o
 NOPED_OBJ_FILES-y += imximage.o
 NOPED_OBJ_FILES-y += omapimage.o
+NOPED_OBJ_FILES-y += mkenvimage.o
 NOPED_OBJ_FILES-y += mkimage.o
 OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o
 NOPED_OBJ_FILES-y += os_support.o
@@ -184,6 +186,9 @@ $(obj)xway-swap-bytes$(SFX):	$(obj)xway-swap-bytes.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
 	$(HOSTSTRIP) $@
 
+$(obj)mkenvimage$(SFX):	$(obj)crc32.o $(obj)mkenvimage.o
+	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
+
 $(obj)mkimage$(SFX):	$(obj)crc32.o \
 			$(obj)default_image.o \
 			$(obj)fit_image.o \
diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
new file mode 100644
index 0000000..8aaa5ce
--- /dev/null
+++ b/tools/mkenvimage.c
@@ -0,0 +1,263 @@
+/*
+ * (C) Copyright 2011 Free Electrons
+ * David Wagner <david.wagner@free-electrons.com>
+ *
+ * Inspired from envcrc.c:
+ * (C) Copyright 2001
+ * Paolo Scaffardi, AIRVENT SAM s.p.a - RIMINI(ITALY), arsenio at tin.it
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+#include <unistd.h>
+#include <compiler.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#include <u-boot/crc.h>
+
+#define CRC_SIZE sizeof(uint32_t)
+
+static void usage(const char *exec_name)
+{
+	fprintf(stderr, "%s [-h] [-r] [-b] [-p <byte>] "
+	       "-s <envrionment partition size> -o <output> <input file>\n"
+	       "\n"
+	       "This tool takes a key=value input file (same as would a "
+	       "`printenv' show) and generates the corresponding environment "
+	       "image, ready to be flashed.\n"
+	       "\n"
+	       "\tThe input file is in format:\n"
+	       "\t\tkey1=value1\n"
+	       "\t\tkey2=value2\n"
+	       "\t\t...\n"
+	       "\t-r : the environment has multiple copies in flash\n"
+	       "\t-b : the target is big endian (default is little endian)\n"
+	       "\t-p <byte> : fill the image with <byte> bytes instead of "
+	       "0xff bytes\n"
+	       "\n"
+	       "If the input file is \"-\", data is read from standard input\n",
+	       exec_name);
+}
+
+int main(int argc, char **argv)
+{
+	uint32_t crc, targetendian_crc;
+	const char *txt_filename = NULL, *bin_filename = NULL;
+	int txt_fd, bin_fd;
+	unsigned char *dataptr, *envptr;
+	unsigned char *filebuf;
+	unsigned int envsize, datasize = 0;
+	int bigendian = 0;
+	int redundant = 0;
+	unsigned char padbyte = 0xff;
+
+	int option;
+	int ret = EXIT_SUCCESS;
+
+	struct stat txt_file_stat;
+
+	int fp, ep;
+
+	/* Parse the cmdline */
+	while ((option = getopt(argc, argv, "s:o:rbp:h")) != -1) {
+		switch (option) {
+		case 's':
+			datasize = strtol(optarg, NULL, 0);
+			break;
+		case 'o':
+			bin_filename = strdup(optarg);
+			if (!bin_filename) {
+				fprintf(stderr, "Can't strdup() the output "
+						"filename\n");
+				return EXIT_FAILURE;
+			}
+			break;
+		case 'r':
+			redundant = 1;
+			break;
+		case 'b':
+			bigendian = 1;
+			break;
+		case 'p':
+			padbyte = strtol(optarg, NULL, 0);
+			break;
+		case 'h':
+			usage(argv[0]);
+			return EXIT_SUCCESS;
+		default:
+			fprintf(stderr, "Wrong option -%c\n", option);
+			usage(argv[0]);
+			return EXIT_FAILURE;
+		}
+	}
+
+	/* Check datasize and allocate the data */
+	if (datasize == 0) {
+		fprintf(stderr,
+			"Please specify the size of the envrionnment "
+			"partition.\n");
+		usage(argv[0]);
+		return EXIT_FAILURE;
+	}
+
+	dataptr = malloc(datasize * sizeof(*dataptr));
+	if (!dataptr) {
+		fprintf(stderr, "Can't alloc dataptr.\n");
+		return EXIT_FAILURE;
+	}
+
+	/*
+	 * envptr points to the beginning of the actual environment (after the
+	 * crc and possible `redundant' bit
+	 */
+	envsize = datasize - (CRC_SIZE + redundant);
+	envptr = dataptr + CRC_SIZE + redundant;
+
+	/* Pad the environment with the padding byte */
+	memset(envptr, padbyte, envsize);
+
+	/* Open the input file ... */
+	if (optind >= argc) {
+		fprintf(stderr, "Please specify an input filename\n");
+		return EXIT_FAILURE;
+	}
+
+	txt_filename = argv[optind];
+	if (strcmp(txt_filename, "-") == 0) {
+		txt_fd = STDIN_FILENO;
+	} else {
+		txt_fd = open(txt_filename, O_RDONLY);
+		if (txt_fd == -1) {
+			fprintf(stderr, "Can't open \"%s\": %s\n",
+					txt_filename, strerror(errno));
+			return EXIT_FAILURE;
+		}
+	}
+	/* ... and check it */
+	ret = fstat(txt_fd, &txt_file_stat);
+	if (ret == -1) {
+		fprintf(stderr, "Can't stat() on \"%s\": "
+				"%s\n", txt_filename, strerror(errno));
+		return EXIT_FAILURE;
+	}
+	/*
+	 * The right test to do is "=>" (not ">") because of the additionnal
+	 * ending \0. See below.
+	 */
+	if (txt_file_stat.st_size >= envsize) {
+		fprintf(stderr, "The input file is larger than the "
+				"envrionnment partition size\n");
+		return EXIT_FAILURE;
+	}
+
+	/* Read the raw input file and transform it */
+	filebuf = malloc(sizeof(*envptr) * txt_file_stat.st_size);
+	ret = read(txt_fd, filebuf, sizeof(*envptr) * txt_file_stat.st_size);
+	if (ret != txt_file_stat.st_size) {
+		fprintf(stderr, "Can't read the whole input file\n");
+		return EXIT_FAILURE;
+	}
+	ret = close(txt_fd);
+
+	/* Replace newlines separating variables with \0 */
+	for (fp = 0, ep = 0 ; fp < txt_file_stat.st_size ; fp++) {
+		/* TODO: what happens if we find a \n at the beginning of the
+		 * file ?
+		 */
+		if (filebuf[fp] == '\n') {
+			if (fp == 0) {
+				/*
+				 * Newline@the beggining of the file ?
+				 * Ignore it.
+				 */
+				continue;
+			} else if (filebuf[fp-1] == '\\') {
+				/*
+				 * Embedded newline in a variable.
+				 *
+				 * The backslash was added to the envptr ;
+				 * rewind and replace it with a newline
+				 */
+				ep--;
+				envptr[ep++] = '\n';
+			} else {
+				/* End of a variable */
+				envptr[ep++] = '\0';
+			}
+		} else if (filebuf[fp] == '#') {
+			if (fp != 0 && filebuf[fp-1] == '\n') {
+				/* This line is a comment, let's skip it */
+				while (fp < txt_file_stat.st_size && fp++ &&
+				       filebuf[fp] != '\n');
+			} else {
+				envptr[ep++] = filebuf[fp];
+			}
+		} else {
+			envptr[ep++] = filebuf[fp];
+		}
+	}
+	/*
+	 * Make sure there is a final '\0'
+	 * And do it again on the next byte to mark the end of the environment.
+	 */
+	if (envptr[ep-1] != '\0') {
+		envptr[ep++] = '\0';
+		/*
+		 * The text file doesn't have an ending newline.  We need to
+		 * check the env size again to make sure we have room for two \0
+		 */
+		if (ep >= envsize) {
+			fprintf(stderr, "The environment file is too large for "
+					"the target environment storage\n");
+			return EXIT_FAILURE;
+		}
+		envptr[ep] = '\0';
+	} else {
+		envptr[ep] = '\0';
+	}
+
+	/* Computes the CRC and put it at the beginning of the data */
+	crc = crc32(0, envptr, envsize);
+	targetendian_crc = bigendian ? cpu_to_be32(crc) : cpu_to_le32(crc);
+
+	memcpy(dataptr, &targetendian_crc, sizeof(uint32_t));
+
+	bin_fd = creat(bin_filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
+	if (bin_fd == -1) {
+		fprintf(stderr, "Can't open output file \"%s\": %s\n",
+				bin_filename, strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	if (write(bin_fd, dataptr, sizeof(*dataptr) * datasize) !=
+			sizeof(*dataptr) * datasize) {
+		fprintf(stderr, "write() failed: %s\n", strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	ret = close(bin_fd);
+
+	return ret;
+}
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv5] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-09-01 15:57 ` [U-Boot] [PATCHv5] " David Wagner
@ 2011-09-01 17:47   ` Mike Frysinger
  0 siblings, 0 replies; 31+ messages in thread
From: Mike Frysinger @ 2011-09-01 17:47 UTC (permalink / raw)
  To: u-boot

On Thursday, September 01, 2011 11:57:09 David Wagner wrote:
> +	txt_filename = argv[optind];
> +	if (strcmp(txt_filename, "-") == 0) {
> +		txt_fd = STDIN_FILENO;
> +	} else {
> +		txt_fd = open(txt_filename, O_RDONLY);
> +		if (txt_fd == -1) {
> +			fprintf(stderr, "Can't open \"%s\": %s\n",
> +					txt_filename, strerror(errno));
> +			return EXIT_FAILURE;
> +		}
> +	}
> +	/* ... and check it */
> +	ret = fstat(txt_fd, &txt_file_stat);
> +	if (ret == -1) {
> +		fprintf(stderr, "Can't stat() on \"%s\": "
> +				"%s\n", txt_filename, strerror(errno));
> +		return EXIT_FAILURE;
> +	}
> +	/*
> +	 * The right test to do is "=>" (not ">") because of the additionnal
> +	 * ending \0. See below.
> +	 */
> +	if (txt_file_stat.st_size >= envsize) {
> +		fprintf(stderr, "The input file is larger than the "
> +				"envrionnment partition size\n");
> +		return EXIT_FAILURE;
> +	}
> +
> +	/* Read the raw input file and transform it */
> +	filebuf = malloc(sizeof(*envptr) * txt_file_stat.st_size);
> +	ret = read(txt_fd, filebuf, sizeof(*envptr) * txt_file_stat.st_size);
> +	if (ret != txt_file_stat.st_size) {
> +		fprintf(stderr, "Can't read the whole input file\n");
> +		return EXIT_FAILURE;
> +	}

i think the stdin logic here is broken.  when you fstat() stdin, you get back 
st_size == 0, and so you end up doing a read() on stdin of 0 bytes, and it 
writes out an empty image.

simple test:
echo foo=var | ./mkenvimage -s 0x1000 -o foo -
hexdump -C foo
<see that the env is empty>

running it through strace shows the bum read(0, "", 0) ...
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110901/5e00bdeb/attachment.pgp 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-05 14:49 [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file David Wagner
                   ` (4 preceding siblings ...)
  2011-09-01 15:57 ` [U-Boot] [PATCHv5] " David Wagner
@ 2011-09-02  8:47 ` David Wagner
  2011-09-02  8:48   ` David Wagner
  2011-09-02  8:48 ` [U-Boot] [PATCHv6] " David Wagner
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 31+ messages in thread
From: David Wagner @ 2011-09-02  8:47 UTC (permalink / raw)
  To: u-boot

This tool takes a key=value configuration file (same as would a `printenv' show)
and generates the corresponding environment image, ready to be flashed.

use case: flash the environment with an external tool

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---

	changes since v5
	~~~~~~~~~~~~~~~~

 - fix reading from stdin

 - remove an obsolete TODO

 tools/Makefile     |    5 +
 tools/mkenvimage.c |  270 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 275 insertions(+), 0 deletions(-)
 create mode 100644 tools/mkenvimage.c

diff --git a/tools/Makefile b/tools/Makefile
index fc741d3..da7caf0 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -66,6 +66,7 @@ BIN_FILES-$(CONFIG_BUILD_ENVCRC) += envcrc$(SFX)
 BIN_FILES-$(CONFIG_CMD_NET) += gen_eth_addr$(SFX)
 BIN_FILES-$(CONFIG_CMD_LOADS) += img2srec$(SFX)
 BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX)
+BIN_FILES-y += mkenvimage$(SFX)
 BIN_FILES-y += mkimage$(SFX)
 BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX)
 BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
@@ -89,6 +90,7 @@ OBJ_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes.o
 NOPED_OBJ_FILES-y += kwbimage.o
 NOPED_OBJ_FILES-y += imximage.o
 NOPED_OBJ_FILES-y += omapimage.o
+NOPED_OBJ_FILES-y += mkenvimage.o
 NOPED_OBJ_FILES-y += mkimage.o
 OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o
 NOPED_OBJ_FILES-y += os_support.o
@@ -184,6 +186,9 @@ $(obj)xway-swap-bytes$(SFX):	$(obj)xway-swap-bytes.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
 	$(HOSTSTRIP) $@
 
+$(obj)mkenvimage$(SFX):	$(obj)crc32.o $(obj)mkenvimage.o
+	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
+
 $(obj)mkimage$(SFX):	$(obj)crc32.o \
 			$(obj)default_image.o \
 			$(obj)fit_image.o \
diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
new file mode 100644
index 0000000..c2636fd
--- /dev/null
+++ b/tools/mkenvimage.c
@@ -0,0 +1,270 @@
+/*
+ * (C) Copyright 2011 Free Electrons
+ * David Wagner <david.wagner@free-electrons.com>
+ *
+ * Inspired from envcrc.c:
+ * (C) Copyright 2001
+ * Paolo Scaffardi, AIRVENT SAM s.p.a - RIMINI(ITALY), arsenio at tin.it
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+#include <unistd.h>
+#include <compiler.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#include <u-boot/crc.h>
+
+#define CRC_SIZE sizeof(uint32_t)
+
+static void usage(const char *exec_name)
+{
+	fprintf(stderr, "%s [-h] [-r] [-b] [-p <byte>] "
+	       "-s <envrionment partition size> -o <output> <input file>\n"
+	       "\n"
+	       "This tool takes a key=value input file (same as would a "
+	       "`printenv' show) and generates the corresponding environment "
+	       "image, ready to be flashed.\n"
+	       "\n"
+	       "\tThe input file is in format:\n"
+	       "\t\tkey1=value1\n"
+	       "\t\tkey2=value2\n"
+	       "\t\t...\n"
+	       "\t-r : the environment has multiple copies in flash\n"
+	       "\t-b : the target is big endian (default is little endian)\n"
+	       "\t-p <byte> : fill the image with <byte> bytes instead of "
+	       "0xff bytes\n"
+	       "\n"
+	       "If the input file is \"-\", data is read from standard input\n",
+	       exec_name);
+}
+
+int main(int argc, char **argv)
+{
+	uint32_t crc, targetendian_crc;
+	const char *txt_filename = NULL, *bin_filename = NULL;
+	int txt_fd, bin_fd;
+	unsigned char *dataptr, *envptr;
+	unsigned char *filebuf = NULL;
+	unsigned int filesize = 0, envsize = 0, datasize = 0;
+	int bigendian = 0;
+	int redundant = 0;
+	unsigned char padbyte = 0xff;
+
+	int option;
+	int ret = EXIT_SUCCESS;
+
+	struct stat txt_file_stat;
+
+	int fp, ep;
+
+	/* Parse the cmdline */
+	while ((option = getopt(argc, argv, "s:o:rbp:h")) != -1) {
+		switch (option) {
+		case 's':
+			datasize = strtol(optarg, NULL, 0);
+			break;
+		case 'o':
+			bin_filename = strdup(optarg);
+			if (!bin_filename) {
+				fprintf(stderr, "Can't strdup() the output "
+						"filename\n");
+				return EXIT_FAILURE;
+			}
+			break;
+		case 'r':
+			redundant = 1;
+			break;
+		case 'b':
+			bigendian = 1;
+			break;
+		case 'p':
+			padbyte = strtol(optarg, NULL, 0);
+			break;
+		case 'h':
+			usage(argv[0]);
+			return EXIT_SUCCESS;
+		default:
+			fprintf(stderr, "Wrong option -%c\n", option);
+			usage(argv[0]);
+			return EXIT_FAILURE;
+		}
+	}
+
+	/* Check datasize and allocate the data */
+	if (datasize == 0) {
+		fprintf(stderr,
+			"Please specify the size of the envrionnment "
+			"partition.\n");
+		usage(argv[0]);
+		return EXIT_FAILURE;
+	}
+
+	dataptr = malloc(datasize * sizeof(*dataptr));
+	if (!dataptr) {
+		fprintf(stderr, "Can't alloc dataptr.\n");
+		return EXIT_FAILURE;
+	}
+
+	/*
+	 * envptr points to the beginning of the actual environment (after the
+	 * crc and possible `redundant' bit
+	 */
+	envsize = datasize - (CRC_SIZE + redundant);
+	envptr = dataptr + CRC_SIZE + redundant;
+
+	/* Pad the environment with the padding byte */
+	memset(envptr, padbyte, envsize);
+
+	/* Open the input file ... */
+	if (optind >= argc) {
+		fprintf(stderr, "Please specify an input filename\n");
+		return EXIT_FAILURE;
+	}
+
+	txt_filename = argv[optind];
+	if (strcmp(txt_filename, "-") == 0) {
+		int readbytes = 0;
+		int readlen = sizeof(*envptr) * 2048;
+		txt_fd = STDIN_FILENO;
+
+		do {
+			filebuf = realloc(filebuf, readlen);
+			readbytes = read(txt_fd, filebuf + filesize, readlen);
+			filesize += readbytes;
+		} while (readbytes == readlen);
+
+	} else {
+		txt_fd = open(txt_filename, O_RDONLY);
+		if (txt_fd == -1) {
+			fprintf(stderr, "Can't open \"%s\": %s\n",
+					txt_filename, strerror(errno));
+			return EXIT_FAILURE;
+		}
+		/* ... and check it */
+		ret = fstat(txt_fd, &txt_file_stat);
+		if (ret == -1) {
+			fprintf(stderr, "Can't stat() on \"%s\": "
+					"%s\n", txt_filename, strerror(errno));
+			return EXIT_FAILURE;
+		}
+
+		filesize = txt_file_stat.st_size;
+		/* Read the raw input file and transform it */
+		filebuf = malloc(sizeof(*envptr) * filesize);
+		ret = read(txt_fd, filebuf, sizeof(*envptr) * filesize);
+		if (ret != sizeof(*envptr) * filesize) {
+			fprintf(stderr, "Can't read the whole input file\n");
+			return EXIT_FAILURE;
+		}
+		ret = close(txt_fd);
+	}
+	/*
+	 * The right test to do is "=>" (not ">") because of the additionnal
+	 * ending \0. See below.
+	 */
+	if (filesize >= envsize) {
+		fprintf(stderr, "The input file is larger than the "
+				"envrionnment partition size\n");
+		return EXIT_FAILURE;
+	}
+
+	/* Replace newlines separating variables with \0 */
+	for (fp = 0, ep = 0 ; fp < filesize ; fp++) {
+		if (filebuf[fp] == '\n') {
+			if (fp == 0) {
+				/*
+				 * Newline@the beggining of the file ?
+				 * Ignore it.
+				 */
+				continue;
+			} else if (filebuf[fp-1] == '\\') {
+				/*
+				 * Embedded newline in a variable.
+				 *
+				 * The backslash was added to the envptr ;
+				 * rewind and replace it with a newline
+				 */
+				ep--;
+				envptr[ep++] = '\n';
+			} else {
+				/* End of a variable */
+				envptr[ep++] = '\0';
+			}
+		} else if (filebuf[fp] == '#') {
+			if (fp != 0 && filebuf[fp-1] == '\n') {
+				/* This line is a comment, let's skip it */
+				while (fp < txt_file_stat.st_size && fp++ &&
+				       filebuf[fp] != '\n');
+			} else {
+				envptr[ep++] = filebuf[fp];
+			}
+		} else {
+			envptr[ep++] = filebuf[fp];
+		}
+	}
+	/*
+	 * Make sure there is a final '\0'
+	 * And do it again on the next byte to mark the end of the environment.
+	 */
+	if (envptr[ep-1] != '\0') {
+		envptr[ep++] = '\0';
+		/*
+		 * The text file doesn't have an ending newline.  We need to
+		 * check the env size again to make sure we have room for two \0
+		 */
+		if (ep >= envsize) {
+			fprintf(stderr, "The environment file is too large for "
+					"the target environment storage\n");
+			return EXIT_FAILURE;
+		}
+		envptr[ep] = '\0';
+	} else {
+		envptr[ep] = '\0';
+	}
+
+	/* Computes the CRC and put it at the beginning of the data */
+	crc = crc32(0, envptr, envsize);
+	targetendian_crc = bigendian ? cpu_to_be32(crc) : cpu_to_le32(crc);
+
+	memcpy(dataptr, &targetendian_crc, sizeof(uint32_t));
+
+	bin_fd = creat(bin_filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
+	if (bin_fd == -1) {
+		fprintf(stderr, "Can't open output file \"%s\": %s\n",
+				bin_filename, strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	if (write(bin_fd, dataptr, sizeof(*dataptr) * datasize) !=
+			sizeof(*dataptr) * datasize) {
+		fprintf(stderr, "write() failed: %s\n", strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	ret = close(bin_fd);
+
+	return ret;
+}
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv6] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-05 14:49 [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file David Wagner
                   ` (5 preceding siblings ...)
  2011-09-02  8:47 ` [U-Boot] [PATCH] " David Wagner
@ 2011-09-02  8:48 ` David Wagner
  2011-09-02 14:35   ` Mike Frysinger
  2011-09-26 12:10   ` Thomas Petazzoni
  2011-09-26 13:26 ` [U-Boot] [PATCHv7] " David Wagner
                   ` (3 subsequent siblings)
  10 siblings, 2 replies; 31+ messages in thread
From: David Wagner @ 2011-09-02  8:48 UTC (permalink / raw)
  To: u-boot

This tool takes a key=value configuration file (same as would a `printenv' show)
and generates the corresponding environment image, ready to be flashed.

use case: flash the environment with an external tool

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---

	changes since v5
	~~~~~~~~~~~~~~~~

 - fix reading from stdin

 - remove an obsolete TODO

 tools/Makefile     |    5 +
 tools/mkenvimage.c |  270 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 275 insertions(+), 0 deletions(-)
 create mode 100644 tools/mkenvimage.c

diff --git a/tools/Makefile b/tools/Makefile
index fc741d3..da7caf0 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -66,6 +66,7 @@ BIN_FILES-$(CONFIG_BUILD_ENVCRC) += envcrc$(SFX)
 BIN_FILES-$(CONFIG_CMD_NET) += gen_eth_addr$(SFX)
 BIN_FILES-$(CONFIG_CMD_LOADS) += img2srec$(SFX)
 BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX)
+BIN_FILES-y += mkenvimage$(SFX)
 BIN_FILES-y += mkimage$(SFX)
 BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX)
 BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
@@ -89,6 +90,7 @@ OBJ_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes.o
 NOPED_OBJ_FILES-y += kwbimage.o
 NOPED_OBJ_FILES-y += imximage.o
 NOPED_OBJ_FILES-y += omapimage.o
+NOPED_OBJ_FILES-y += mkenvimage.o
 NOPED_OBJ_FILES-y += mkimage.o
 OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o
 NOPED_OBJ_FILES-y += os_support.o
@@ -184,6 +186,9 @@ $(obj)xway-swap-bytes$(SFX):	$(obj)xway-swap-bytes.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
 	$(HOSTSTRIP) $@
 
+$(obj)mkenvimage$(SFX):	$(obj)crc32.o $(obj)mkenvimage.o
+	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
+
 $(obj)mkimage$(SFX):	$(obj)crc32.o \
 			$(obj)default_image.o \
 			$(obj)fit_image.o \
diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
new file mode 100644
index 0000000..c2636fd
--- /dev/null
+++ b/tools/mkenvimage.c
@@ -0,0 +1,270 @@
+/*
+ * (C) Copyright 2011 Free Electrons
+ * David Wagner <david.wagner@free-electrons.com>
+ *
+ * Inspired from envcrc.c:
+ * (C) Copyright 2001
+ * Paolo Scaffardi, AIRVENT SAM s.p.a - RIMINI(ITALY), arsenio at tin.it
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+#include <unistd.h>
+#include <compiler.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#include <u-boot/crc.h>
+
+#define CRC_SIZE sizeof(uint32_t)
+
+static void usage(const char *exec_name)
+{
+	fprintf(stderr, "%s [-h] [-r] [-b] [-p <byte>] "
+	       "-s <envrionment partition size> -o <output> <input file>\n"
+	       "\n"
+	       "This tool takes a key=value input file (same as would a "
+	       "`printenv' show) and generates the corresponding environment "
+	       "image, ready to be flashed.\n"
+	       "\n"
+	       "\tThe input file is in format:\n"
+	       "\t\tkey1=value1\n"
+	       "\t\tkey2=value2\n"
+	       "\t\t...\n"
+	       "\t-r : the environment has multiple copies in flash\n"
+	       "\t-b : the target is big endian (default is little endian)\n"
+	       "\t-p <byte> : fill the image with <byte> bytes instead of "
+	       "0xff bytes\n"
+	       "\n"
+	       "If the input file is \"-\", data is read from standard input\n",
+	       exec_name);
+}
+
+int main(int argc, char **argv)
+{
+	uint32_t crc, targetendian_crc;
+	const char *txt_filename = NULL, *bin_filename = NULL;
+	int txt_fd, bin_fd;
+	unsigned char *dataptr, *envptr;
+	unsigned char *filebuf = NULL;
+	unsigned int filesize = 0, envsize = 0, datasize = 0;
+	int bigendian = 0;
+	int redundant = 0;
+	unsigned char padbyte = 0xff;
+
+	int option;
+	int ret = EXIT_SUCCESS;
+
+	struct stat txt_file_stat;
+
+	int fp, ep;
+
+	/* Parse the cmdline */
+	while ((option = getopt(argc, argv, "s:o:rbp:h")) != -1) {
+		switch (option) {
+		case 's':
+			datasize = strtol(optarg, NULL, 0);
+			break;
+		case 'o':
+			bin_filename = strdup(optarg);
+			if (!bin_filename) {
+				fprintf(stderr, "Can't strdup() the output "
+						"filename\n");
+				return EXIT_FAILURE;
+			}
+			break;
+		case 'r':
+			redundant = 1;
+			break;
+		case 'b':
+			bigendian = 1;
+			break;
+		case 'p':
+			padbyte = strtol(optarg, NULL, 0);
+			break;
+		case 'h':
+			usage(argv[0]);
+			return EXIT_SUCCESS;
+		default:
+			fprintf(stderr, "Wrong option -%c\n", option);
+			usage(argv[0]);
+			return EXIT_FAILURE;
+		}
+	}
+
+	/* Check datasize and allocate the data */
+	if (datasize == 0) {
+		fprintf(stderr,
+			"Please specify the size of the envrionnment "
+			"partition.\n");
+		usage(argv[0]);
+		return EXIT_FAILURE;
+	}
+
+	dataptr = malloc(datasize * sizeof(*dataptr));
+	if (!dataptr) {
+		fprintf(stderr, "Can't alloc dataptr.\n");
+		return EXIT_FAILURE;
+	}
+
+	/*
+	 * envptr points to the beginning of the actual environment (after the
+	 * crc and possible `redundant' bit
+	 */
+	envsize = datasize - (CRC_SIZE + redundant);
+	envptr = dataptr + CRC_SIZE + redundant;
+
+	/* Pad the environment with the padding byte */
+	memset(envptr, padbyte, envsize);
+
+	/* Open the input file ... */
+	if (optind >= argc) {
+		fprintf(stderr, "Please specify an input filename\n");
+		return EXIT_FAILURE;
+	}
+
+	txt_filename = argv[optind];
+	if (strcmp(txt_filename, "-") == 0) {
+		int readbytes = 0;
+		int readlen = sizeof(*envptr) * 2048;
+		txt_fd = STDIN_FILENO;
+
+		do {
+			filebuf = realloc(filebuf, readlen);
+			readbytes = read(txt_fd, filebuf + filesize, readlen);
+			filesize += readbytes;
+		} while (readbytes == readlen);
+
+	} else {
+		txt_fd = open(txt_filename, O_RDONLY);
+		if (txt_fd == -1) {
+			fprintf(stderr, "Can't open \"%s\": %s\n",
+					txt_filename, strerror(errno));
+			return EXIT_FAILURE;
+		}
+		/* ... and check it */
+		ret = fstat(txt_fd, &txt_file_stat);
+		if (ret == -1) {
+			fprintf(stderr, "Can't stat() on \"%s\": "
+					"%s\n", txt_filename, strerror(errno));
+			return EXIT_FAILURE;
+		}
+
+		filesize = txt_file_stat.st_size;
+		/* Read the raw input file and transform it */
+		filebuf = malloc(sizeof(*envptr) * filesize);
+		ret = read(txt_fd, filebuf, sizeof(*envptr) * filesize);
+		if (ret != sizeof(*envptr) * filesize) {
+			fprintf(stderr, "Can't read the whole input file\n");
+			return EXIT_FAILURE;
+		}
+		ret = close(txt_fd);
+	}
+	/*
+	 * The right test to do is "=>" (not ">") because of the additionnal
+	 * ending \0. See below.
+	 */
+	if (filesize >= envsize) {
+		fprintf(stderr, "The input file is larger than the "
+				"envrionnment partition size\n");
+		return EXIT_FAILURE;
+	}
+
+	/* Replace newlines separating variables with \0 */
+	for (fp = 0, ep = 0 ; fp < filesize ; fp++) {
+		if (filebuf[fp] == '\n') {
+			if (fp == 0) {
+				/*
+				 * Newline@the beggining of the file ?
+				 * Ignore it.
+				 */
+				continue;
+			} else if (filebuf[fp-1] == '\\') {
+				/*
+				 * Embedded newline in a variable.
+				 *
+				 * The backslash was added to the envptr ;
+				 * rewind and replace it with a newline
+				 */
+				ep--;
+				envptr[ep++] = '\n';
+			} else {
+				/* End of a variable */
+				envptr[ep++] = '\0';
+			}
+		} else if (filebuf[fp] == '#') {
+			if (fp != 0 && filebuf[fp-1] == '\n') {
+				/* This line is a comment, let's skip it */
+				while (fp < txt_file_stat.st_size && fp++ &&
+				       filebuf[fp] != '\n');
+			} else {
+				envptr[ep++] = filebuf[fp];
+			}
+		} else {
+			envptr[ep++] = filebuf[fp];
+		}
+	}
+	/*
+	 * Make sure there is a final '\0'
+	 * And do it again on the next byte to mark the end of the environment.
+	 */
+	if (envptr[ep-1] != '\0') {
+		envptr[ep++] = '\0';
+		/*
+		 * The text file doesn't have an ending newline.  We need to
+		 * check the env size again to make sure we have room for two \0
+		 */
+		if (ep >= envsize) {
+			fprintf(stderr, "The environment file is too large for "
+					"the target environment storage\n");
+			return EXIT_FAILURE;
+		}
+		envptr[ep] = '\0';
+	} else {
+		envptr[ep] = '\0';
+	}
+
+	/* Computes the CRC and put it at the beginning of the data */
+	crc = crc32(0, envptr, envsize);
+	targetendian_crc = bigendian ? cpu_to_be32(crc) : cpu_to_le32(crc);
+
+	memcpy(dataptr, &targetendian_crc, sizeof(uint32_t));
+
+	bin_fd = creat(bin_filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
+	if (bin_fd == -1) {
+		fprintf(stderr, "Can't open output file \"%s\": %s\n",
+				bin_filename, strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	if (write(bin_fd, dataptr, sizeof(*dataptr) * datasize) !=
+			sizeof(*dataptr) * datasize) {
+		fprintf(stderr, "write() failed: %s\n", strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	ret = close(bin_fd);
+
+	return ret;
+}
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-09-02  8:47 ` [U-Boot] [PATCH] " David Wagner
@ 2011-09-02  8:48   ` David Wagner
  0 siblings, 0 replies; 31+ messages in thread
From: David Wagner @ 2011-09-02  8:48 UTC (permalink / raw)
  To: u-boot

oops, subject is missing "v6". I just resent a correct version.

David

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv6] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-09-02  8:48 ` [U-Boot] [PATCHv6] " David Wagner
@ 2011-09-02 14:35   ` Mike Frysinger
  2011-09-26 12:10   ` Thomas Petazzoni
  1 sibling, 0 replies; 31+ messages in thread
From: Mike Frysinger @ 2011-09-02 14:35 UTC (permalink / raw)
  To: u-boot

On Friday, September 02, 2011 04:48:03 David Wagner wrote:
> This tool takes a key=value configuration file (same as would a `printenv'
> show) and generates the corresponding environment image, ready to be
> flashed.
> 
> use case: flash the environment with an external tool

Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110902/53553ad6/attachment.pgp 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv6] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-09-02  8:48 ` [U-Boot] [PATCHv6] " David Wagner
  2011-09-02 14:35   ` Mike Frysinger
@ 2011-09-26 12:10   ` Thomas Petazzoni
  1 sibling, 0 replies; 31+ messages in thread
From: Thomas Petazzoni @ 2011-09-26 12:10 UTC (permalink / raw)
  To: u-boot

Hello,

Le Fri,  2 Sep 2011 10:48:03 +0200,
David Wagner <david.wagner@free-electrons.com> a ?crit :

> +	       "-s <envrionment partition size> -o <output> <input file>\n"

                   ^^^^^^ typo

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv7] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-05 14:49 [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file David Wagner
                   ` (6 preceding siblings ...)
  2011-09-02  8:48 ` [U-Boot] [PATCHv6] " David Wagner
@ 2011-09-26 13:26 ` David Wagner
  2011-09-30  7:31 ` [U-Boot] [PATCHv8] " David Wagner
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 31+ messages in thread
From: David Wagner @ 2011-09-26 13:26 UTC (permalink / raw)
  To: u-boot

This tool takes a key=value configuration file (same as would a `printenv' show)
and generates the corresponding environment image, ready to be flashed.

use case: flash the environment with an external tool

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
Acked-by; Mike Frysinger <vapier@gentoo.org>
Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---

	changes since v6:
	~~~~~~~~~~~~~~~~~

 * fix a typo
 * add Mike's Acked-by and Thomas' Tested-by

 tools/Makefile     |    5 +
 tools/mkenvimage.c |  270 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 275 insertions(+), 0 deletions(-)
 create mode 100644 tools/mkenvimage.c

diff --git a/tools/Makefile b/tools/Makefile
index fc741d3..da7caf0 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -66,6 +66,7 @@ BIN_FILES-$(CONFIG_BUILD_ENVCRC) += envcrc$(SFX)
 BIN_FILES-$(CONFIG_CMD_NET) += gen_eth_addr$(SFX)
 BIN_FILES-$(CONFIG_CMD_LOADS) += img2srec$(SFX)
 BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX)
+BIN_FILES-y += mkenvimage$(SFX)
 BIN_FILES-y += mkimage$(SFX)
 BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX)
 BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
@@ -89,6 +90,7 @@ OBJ_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes.o
 NOPED_OBJ_FILES-y += kwbimage.o
 NOPED_OBJ_FILES-y += imximage.o
 NOPED_OBJ_FILES-y += omapimage.o
+NOPED_OBJ_FILES-y += mkenvimage.o
 NOPED_OBJ_FILES-y += mkimage.o
 OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o
 NOPED_OBJ_FILES-y += os_support.o
@@ -184,6 +186,9 @@ $(obj)xway-swap-bytes$(SFX):	$(obj)xway-swap-bytes.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
 	$(HOSTSTRIP) $@
 
+$(obj)mkenvimage$(SFX):	$(obj)crc32.o $(obj)mkenvimage.o
+	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
+
 $(obj)mkimage$(SFX):	$(obj)crc32.o \
 			$(obj)default_image.o \
 			$(obj)fit_image.o \
diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
new file mode 100644
index 0000000..9c32f4a
--- /dev/null
+++ b/tools/mkenvimage.c
@@ -0,0 +1,270 @@
+/*
+ * (C) Copyright 2011 Free Electrons
+ * David Wagner <david.wagner@free-electrons.com>
+ *
+ * Inspired from envcrc.c:
+ * (C) Copyright 2001
+ * Paolo Scaffardi, AIRVENT SAM s.p.a - RIMINI(ITALY), arsenio at tin.it
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+#include <unistd.h>
+#include <compiler.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#include <u-boot/crc.h>
+
+#define CRC_SIZE sizeof(uint32_t)
+
+static void usage(const char *exec_name)
+{
+	fprintf(stderr, "%s [-h] [-r] [-b] [-p <byte>] "
+	       "-s <environment partition size> -o <output> <input file>\n"
+	       "\n"
+	       "This tool takes a key=value input file (same as would a "
+	       "`printenv' show) and generates the corresponding environment "
+	       "image, ready to be flashed.\n"
+	       "\n"
+	       "\tThe input file is in format:\n"
+	       "\t\tkey1=value1\n"
+	       "\t\tkey2=value2\n"
+	       "\t\t...\n"
+	       "\t-r : the environment has multiple copies in flash\n"
+	       "\t-b : the target is big endian (default is little endian)\n"
+	       "\t-p <byte> : fill the image with <byte> bytes instead of "
+	       "0xff bytes\n"
+	       "\n"
+	       "If the input file is \"-\", data is read from standard input\n",
+	       exec_name);
+}
+
+int main(int argc, char **argv)
+{
+	uint32_t crc, targetendian_crc;
+	const char *txt_filename = NULL, *bin_filename = NULL;
+	int txt_fd, bin_fd;
+	unsigned char *dataptr, *envptr;
+	unsigned char *filebuf = NULL;
+	unsigned int filesize = 0, envsize = 0, datasize = 0;
+	int bigendian = 0;
+	int redundant = 0;
+	unsigned char padbyte = 0xff;
+
+	int option;
+	int ret = EXIT_SUCCESS;
+
+	struct stat txt_file_stat;
+
+	int fp, ep;
+
+	/* Parse the cmdline */
+	while ((option = getopt(argc, argv, "s:o:rbp:h")) != -1) {
+		switch (option) {
+		case 's':
+			datasize = strtol(optarg, NULL, 0);
+			break;
+		case 'o':
+			bin_filename = strdup(optarg);
+			if (!bin_filename) {
+				fprintf(stderr, "Can't strdup() the output "
+						"filename\n");
+				return EXIT_FAILURE;
+			}
+			break;
+		case 'r':
+			redundant = 1;
+			break;
+		case 'b':
+			bigendian = 1;
+			break;
+		case 'p':
+			padbyte = strtol(optarg, NULL, 0);
+			break;
+		case 'h':
+			usage(argv[0]);
+			return EXIT_SUCCESS;
+		default:
+			fprintf(stderr, "Wrong option -%c\n", option);
+			usage(argv[0]);
+			return EXIT_FAILURE;
+		}
+	}
+
+	/* Check datasize and allocate the data */
+	if (datasize == 0) {
+		fprintf(stderr,
+			"Please specify the size of the envrionnment "
+			"partition.\n");
+		usage(argv[0]);
+		return EXIT_FAILURE;
+	}
+
+	dataptr = malloc(datasize * sizeof(*dataptr));
+	if (!dataptr) {
+		fprintf(stderr, "Can't alloc dataptr.\n");
+		return EXIT_FAILURE;
+	}
+
+	/*
+	 * envptr points to the beginning of the actual environment (after the
+	 * crc and possible `redundant' bit
+	 */
+	envsize = datasize - (CRC_SIZE + redundant);
+	envptr = dataptr + CRC_SIZE + redundant;
+
+	/* Pad the environment with the padding byte */
+	memset(envptr, padbyte, envsize);
+
+	/* Open the input file ... */
+	if (optind >= argc) {
+		fprintf(stderr, "Please specify an input filename\n");
+		return EXIT_FAILURE;
+	}
+
+	txt_filename = argv[optind];
+	if (strcmp(txt_filename, "-") == 0) {
+		int readbytes = 0;
+		int readlen = sizeof(*envptr) * 2048;
+		txt_fd = STDIN_FILENO;
+
+		do {
+			filebuf = realloc(filebuf, readlen);
+			readbytes = read(txt_fd, filebuf + filesize, readlen);
+			filesize += readbytes;
+		} while (readbytes == readlen);
+
+	} else {
+		txt_fd = open(txt_filename, O_RDONLY);
+		if (txt_fd == -1) {
+			fprintf(stderr, "Can't open \"%s\": %s\n",
+					txt_filename, strerror(errno));
+			return EXIT_FAILURE;
+		}
+		/* ... and check it */
+		ret = fstat(txt_fd, &txt_file_stat);
+		if (ret == -1) {
+			fprintf(stderr, "Can't stat() on \"%s\": "
+					"%s\n", txt_filename, strerror(errno));
+			return EXIT_FAILURE;
+		}
+
+		filesize = txt_file_stat.st_size;
+		/* Read the raw input file and transform it */
+		filebuf = malloc(sizeof(*envptr) * filesize);
+		ret = read(txt_fd, filebuf, sizeof(*envptr) * filesize);
+		if (ret != sizeof(*envptr) * filesize) {
+			fprintf(stderr, "Can't read the whole input file\n");
+			return EXIT_FAILURE;
+		}
+		ret = close(txt_fd);
+	}
+	/*
+	 * The right test to do is "=>" (not ">") because of the additionnal
+	 * ending \0. See below.
+	 */
+	if (filesize >= envsize) {
+		fprintf(stderr, "The input file is larger than the "
+				"envrionnment partition size\n");
+		return EXIT_FAILURE;
+	}
+
+	/* Replace newlines separating variables with \0 */
+	for (fp = 0, ep = 0 ; fp < filesize ; fp++) {
+		if (filebuf[fp] == '\n') {
+			if (fp == 0) {
+				/*
+				 * Newline@the beggining of the file ?
+				 * Ignore it.
+				 */
+				continue;
+			} else if (filebuf[fp-1] == '\\') {
+				/*
+				 * Embedded newline in a variable.
+				 *
+				 * The backslash was added to the envptr ;
+				 * rewind and replace it with a newline
+				 */
+				ep--;
+				envptr[ep++] = '\n';
+			} else {
+				/* End of a variable */
+				envptr[ep++] = '\0';
+			}
+		} else if (filebuf[fp] == '#') {
+			if (fp != 0 && filebuf[fp-1] == '\n') {
+				/* This line is a comment, let's skip it */
+				while (fp < txt_file_stat.st_size && fp++ &&
+				       filebuf[fp] != '\n');
+			} else {
+				envptr[ep++] = filebuf[fp];
+			}
+		} else {
+			envptr[ep++] = filebuf[fp];
+		}
+	}
+	/*
+	 * Make sure there is a final '\0'
+	 * And do it again on the next byte to mark the end of the environment.
+	 */
+	if (envptr[ep-1] != '\0') {
+		envptr[ep++] = '\0';
+		/*
+		 * The text file doesn't have an ending newline.  We need to
+		 * check the env size again to make sure we have room for two \0
+		 */
+		if (ep >= envsize) {
+			fprintf(stderr, "The environment file is too large for "
+					"the target environment storage\n");
+			return EXIT_FAILURE;
+		}
+		envptr[ep] = '\0';
+	} else {
+		envptr[ep] = '\0';
+	}
+
+	/* Computes the CRC and put it at the beginning of the data */
+	crc = crc32(0, envptr, envsize);
+	targetendian_crc = bigendian ? cpu_to_be32(crc) : cpu_to_le32(crc);
+
+	memcpy(dataptr, &targetendian_crc, sizeof(uint32_t));
+
+	bin_fd = creat(bin_filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
+	if (bin_fd == -1) {
+		fprintf(stderr, "Can't open output file \"%s\": %s\n",
+				bin_filename, strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	if (write(bin_fd, dataptr, sizeof(*dataptr) * datasize) !=
+			sizeof(*dataptr) * datasize) {
+		fprintf(stderr, "write() failed: %s\n", strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	ret = close(bin_fd);
+
+	return ret;
+}
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv8] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-05 14:49 [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file David Wagner
                   ` (7 preceding siblings ...)
  2011-09-26 13:26 ` [U-Boot] [PATCHv7] " David Wagner
@ 2011-09-30  7:31 ` David Wagner
  2011-10-06 21:17   ` Wolfgang Denk
  2011-10-13 18:45 ` [U-Boot] [PATCHv9] " David Wagner
  2011-10-14 17:16 ` [U-Boot] [PATCHv10] " David Wagner
  10 siblings, 1 reply; 31+ messages in thread
From: David Wagner @ 2011-09-30  7:31 UTC (permalink / raw)
  To: u-boot

This tool takes a key=value configuration file (same as would a `printenv' show)
and generates the corresponding environment image, ready to be flashed.

use case: flash the environment with an external tool

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
Acked-by; Mike Frysinger <vapier@gentoo.org>
Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---

change since v7:

add a missing header (stdlib.h)

 tools/Makefile     |    5 +
 tools/mkenvimage.c |  271 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 276 insertions(+), 0 deletions(-)
 create mode 100644 tools/mkenvimage.c

diff --git a/tools/Makefile b/tools/Makefile
index fc741d3..da7caf0 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -66,6 +66,7 @@ BIN_FILES-$(CONFIG_BUILD_ENVCRC) += envcrc$(SFX)
 BIN_FILES-$(CONFIG_CMD_NET) += gen_eth_addr$(SFX)
 BIN_FILES-$(CONFIG_CMD_LOADS) += img2srec$(SFX)
 BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX)
+BIN_FILES-y += mkenvimage$(SFX)
 BIN_FILES-y += mkimage$(SFX)
 BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX)
 BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
@@ -89,6 +90,7 @@ OBJ_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes.o
 NOPED_OBJ_FILES-y += kwbimage.o
 NOPED_OBJ_FILES-y += imximage.o
 NOPED_OBJ_FILES-y += omapimage.o
+NOPED_OBJ_FILES-y += mkenvimage.o
 NOPED_OBJ_FILES-y += mkimage.o
 OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o
 NOPED_OBJ_FILES-y += os_support.o
@@ -184,6 +186,9 @@ $(obj)xway-swap-bytes$(SFX):	$(obj)xway-swap-bytes.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
 	$(HOSTSTRIP) $@
 
+$(obj)mkenvimage$(SFX):	$(obj)crc32.o $(obj)mkenvimage.o
+	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
+
 $(obj)mkimage$(SFX):	$(obj)crc32.o \
 			$(obj)default_image.o \
 			$(obj)fit_image.o \
diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
new file mode 100644
index 0000000..ad2c8d3
--- /dev/null
+++ b/tools/mkenvimage.c
@@ -0,0 +1,271 @@
+/*
+ * (C) Copyright 2011 Free Electrons
+ * David Wagner <david.wagner@free-electrons.com>
+ *
+ * Inspired from envcrc.c:
+ * (C) Copyright 2001
+ * Paolo Scaffardi, AIRVENT SAM s.p.a - RIMINI(ITALY), arsenio at tin.it
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+#include <unistd.h>
+#include <compiler.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#include <u-boot/crc.h>
+
+#define CRC_SIZE sizeof(uint32_t)
+
+static void usage(const char *exec_name)
+{
+	fprintf(stderr, "%s [-h] [-r] [-b] [-p <byte>] "
+	       "-s <environment partition size> -o <output> <input file>\n"
+	       "\n"
+	       "This tool takes a key=value input file (same as would a "
+	       "`printenv' show) and generates the corresponding environment "
+	       "image, ready to be flashed.\n"
+	       "\n"
+	       "\tThe input file is in format:\n"
+	       "\t\tkey1=value1\n"
+	       "\t\tkey2=value2\n"
+	       "\t\t...\n"
+	       "\t-r : the environment has multiple copies in flash\n"
+	       "\t-b : the target is big endian (default is little endian)\n"
+	       "\t-p <byte> : fill the image with <byte> bytes instead of "
+	       "0xff bytes\n"
+	       "\n"
+	       "If the input file is \"-\", data is read from standard input\n",
+	       exec_name);
+}
+
+int main(int argc, char **argv)
+{
+	uint32_t crc, targetendian_crc;
+	const char *txt_filename = NULL, *bin_filename = NULL;
+	int txt_fd, bin_fd;
+	unsigned char *dataptr, *envptr;
+	unsigned char *filebuf = NULL;
+	unsigned int filesize = 0, envsize = 0, datasize = 0;
+	int bigendian = 0;
+	int redundant = 0;
+	unsigned char padbyte = 0xff;
+
+	int option;
+	int ret = EXIT_SUCCESS;
+
+	struct stat txt_file_stat;
+
+	int fp, ep;
+
+	/* Parse the cmdline */
+	while ((option = getopt(argc, argv, "s:o:rbp:h")) != -1) {
+		switch (option) {
+		case 's':
+			datasize = strtol(optarg, NULL, 0);
+			break;
+		case 'o':
+			bin_filename = strdup(optarg);
+			if (!bin_filename) {
+				fprintf(stderr, "Can't strdup() the output "
+						"filename\n");
+				return EXIT_FAILURE;
+			}
+			break;
+		case 'r':
+			redundant = 1;
+			break;
+		case 'b':
+			bigendian = 1;
+			break;
+		case 'p':
+			padbyte = strtol(optarg, NULL, 0);
+			break;
+		case 'h':
+			usage(argv[0]);
+			return EXIT_SUCCESS;
+		default:
+			fprintf(stderr, "Wrong option -%c\n", option);
+			usage(argv[0]);
+			return EXIT_FAILURE;
+		}
+	}
+
+	/* Check datasize and allocate the data */
+	if (datasize == 0) {
+		fprintf(stderr,
+			"Please specify the size of the envrionnment "
+			"partition.\n");
+		usage(argv[0]);
+		return EXIT_FAILURE;
+	}
+
+	dataptr = malloc(datasize * sizeof(*dataptr));
+	if (!dataptr) {
+		fprintf(stderr, "Can't alloc dataptr.\n");
+		return EXIT_FAILURE;
+	}
+
+	/*
+	 * envptr points to the beginning of the actual environment (after the
+	 * crc and possible `redundant' bit
+	 */
+	envsize = datasize - (CRC_SIZE + redundant);
+	envptr = dataptr + CRC_SIZE + redundant;
+
+	/* Pad the environment with the padding byte */
+	memset(envptr, padbyte, envsize);
+
+	/* Open the input file ... */
+	if (optind >= argc) {
+		fprintf(stderr, "Please specify an input filename\n");
+		return EXIT_FAILURE;
+	}
+
+	txt_filename = argv[optind];
+	if (strcmp(txt_filename, "-") == 0) {
+		int readbytes = 0;
+		int readlen = sizeof(*envptr) * 2048;
+		txt_fd = STDIN_FILENO;
+
+		do {
+			filebuf = realloc(filebuf, readlen);
+			readbytes = read(txt_fd, filebuf + filesize, readlen);
+			filesize += readbytes;
+		} while (readbytes == readlen);
+
+	} else {
+		txt_fd = open(txt_filename, O_RDONLY);
+		if (txt_fd == -1) {
+			fprintf(stderr, "Can't open \"%s\": %s\n",
+					txt_filename, strerror(errno));
+			return EXIT_FAILURE;
+		}
+		/* ... and check it */
+		ret = fstat(txt_fd, &txt_file_stat);
+		if (ret == -1) {
+			fprintf(stderr, "Can't stat() on \"%s\": "
+					"%s\n", txt_filename, strerror(errno));
+			return EXIT_FAILURE;
+		}
+
+		filesize = txt_file_stat.st_size;
+		/* Read the raw input file and transform it */
+		filebuf = malloc(sizeof(*envptr) * filesize);
+		ret = read(txt_fd, filebuf, sizeof(*envptr) * filesize);
+		if (ret != sizeof(*envptr) * filesize) {
+			fprintf(stderr, "Can't read the whole input file\n");
+			return EXIT_FAILURE;
+		}
+		ret = close(txt_fd);
+	}
+	/*
+	 * The right test to do is "=>" (not ">") because of the additionnal
+	 * ending \0. See below.
+	 */
+	if (filesize >= envsize) {
+		fprintf(stderr, "The input file is larger than the "
+				"envrionnment partition size\n");
+		return EXIT_FAILURE;
+	}
+
+	/* Replace newlines separating variables with \0 */
+	for (fp = 0, ep = 0 ; fp < filesize ; fp++) {
+		if (filebuf[fp] == '\n') {
+			if (fp == 0) {
+				/*
+				 * Newline@the beggining of the file ?
+				 * Ignore it.
+				 */
+				continue;
+			} else if (filebuf[fp-1] == '\\') {
+				/*
+				 * Embedded newline in a variable.
+				 *
+				 * The backslash was added to the envptr ;
+				 * rewind and replace it with a newline
+				 */
+				ep--;
+				envptr[ep++] = '\n';
+			} else {
+				/* End of a variable */
+				envptr[ep++] = '\0';
+			}
+		} else if (filebuf[fp] == '#') {
+			if (fp != 0 && filebuf[fp-1] == '\n') {
+				/* This line is a comment, let's skip it */
+				while (fp < txt_file_stat.st_size && fp++ &&
+				       filebuf[fp] != '\n');
+			} else {
+				envptr[ep++] = filebuf[fp];
+			}
+		} else {
+			envptr[ep++] = filebuf[fp];
+		}
+	}
+	/*
+	 * Make sure there is a final '\0'
+	 * And do it again on the next byte to mark the end of the environment.
+	 */
+	if (envptr[ep-1] != '\0') {
+		envptr[ep++] = '\0';
+		/*
+		 * The text file doesn't have an ending newline.  We need to
+		 * check the env size again to make sure we have room for two \0
+		 */
+		if (ep >= envsize) {
+			fprintf(stderr, "The environment file is too large for "
+					"the target environment storage\n");
+			return EXIT_FAILURE;
+		}
+		envptr[ep] = '\0';
+	} else {
+		envptr[ep] = '\0';
+	}
+
+	/* Computes the CRC and put it at the beginning of the data */
+	crc = crc32(0, envptr, envsize);
+	targetendian_crc = bigendian ? cpu_to_be32(crc) : cpu_to_le32(crc);
+
+	memcpy(dataptr, &targetendian_crc, sizeof(uint32_t));
+
+	bin_fd = creat(bin_filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
+	if (bin_fd == -1) {
+		fprintf(stderr, "Can't open output file \"%s\": %s\n",
+				bin_filename, strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	if (write(bin_fd, dataptr, sizeof(*dataptr) * datasize) !=
+			sizeof(*dataptr) * datasize) {
+		fprintf(stderr, "write() failed: %s\n", strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	ret = close(bin_fd);
+
+	return ret;
+}
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv8] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-09-30  7:31 ` [U-Boot] [PATCHv8] " David Wagner
@ 2011-10-06 21:17   ` Wolfgang Denk
  0 siblings, 0 replies; 31+ messages in thread
From: Wolfgang Denk @ 2011-10-06 21:17 UTC (permalink / raw)
  To: u-boot

Dear David Wagner,

In message <1317367880-13986-1-git-send-email-david.wagner@free-electrons.com> you wrote:
> This tool takes a key=value configuration file (same as would a `printenv' show)
> and generates the corresponding environment image, ready to be flashed.
> 
> use case: flash the environment with an external tool
> 
> Signed-off-by: David Wagner <david.wagner@free-electrons.com>
> Acked-by; Mike Frysinger <vapier@gentoo.org>
> Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> 
> change since v7:
> 
> add a missing header (stdlib.h)
> 
>  tools/Makefile     |    5 +
>  tools/mkenvimage.c |  271 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 276 insertions(+), 0 deletions(-)
>  create mode 100644 tools/mkenvimage.c

Checkpatch says:

total: 1 errors, 0 warnings, 294 lines checked

Please clean up and resubmit.  Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I don't care if you *ARE* on a bondage-and-discipline  post-technical
system  pawned off by the nation's largest oughta-be-illegal monopoly
who cannot escape the sins of their forefathers -- namely, using  the
wrong  slash for directories when the C language and its brethren use
it for something else that's very important.
         -- Tom Christiansen in <55oabg$1j1$1@csnews.cs.colorado.edu>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv9] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-05 14:49 [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file David Wagner
                   ` (8 preceding siblings ...)
  2011-09-30  7:31 ` [U-Boot] [PATCHv8] " David Wagner
@ 2011-10-13 18:45 ` David Wagner
  2011-10-14  8:21   ` Detlev Zundel
  2011-10-14 17:16 ` [U-Boot] [PATCHv10] " David Wagner
  10 siblings, 1 reply; 31+ messages in thread
From: David Wagner @ 2011-10-13 18:45 UTC (permalink / raw)
  To: u-boot

This tool takes a key=value configuration file (same as would a `printenv' show)
and generates the corresponding environment image, ready to be flashed.

use case: flash the environment with an external tool

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
Acked-by; Mike Frysinger <vapier@gentoo.org>
Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
(My first try to send it seems to have failed ; please disregard if it actually
suceeded)

	change since v8
	~~~~~~~~~~~~~~~

 * Make checkpatch happy by putting an instruction out of a while().
   Incidently, it also fixes a possible segfault (in the event of a comment
   being at the end of a file that hasn't an ending newline).

 tools/Makefile     |    5 +
 tools/mkenvimage.c |  272 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 277 insertions(+), 0 deletions(-)
 create mode 100644 tools/mkenvimage.c

diff --git a/tools/Makefile b/tools/Makefile
index fc741d3..da7caf0 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -66,6 +66,7 @@ BIN_FILES-$(CONFIG_BUILD_ENVCRC) += envcrc$(SFX)
 BIN_FILES-$(CONFIG_CMD_NET) += gen_eth_addr$(SFX)
 BIN_FILES-$(CONFIG_CMD_LOADS) += img2srec$(SFX)
 BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX)
+BIN_FILES-y += mkenvimage$(SFX)
 BIN_FILES-y += mkimage$(SFX)
 BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX)
 BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
@@ -89,6 +90,7 @@ OBJ_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes.o
 NOPED_OBJ_FILES-y += kwbimage.o
 NOPED_OBJ_FILES-y += imximage.o
 NOPED_OBJ_FILES-y += omapimage.o
+NOPED_OBJ_FILES-y += mkenvimage.o
 NOPED_OBJ_FILES-y += mkimage.o
 OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o
 NOPED_OBJ_FILES-y += os_support.o
@@ -184,6 +186,9 @@ $(obj)xway-swap-bytes$(SFX):	$(obj)xway-swap-bytes.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
 	$(HOSTSTRIP) $@
 
+$(obj)mkenvimage$(SFX):	$(obj)crc32.o $(obj)mkenvimage.o
+	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
+
 $(obj)mkimage$(SFX):	$(obj)crc32.o \
 			$(obj)default_image.o \
 			$(obj)fit_image.o \
diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
new file mode 100644
index 0000000..2c7fbe7
--- /dev/null
+++ b/tools/mkenvimage.c
@@ -0,0 +1,272 @@
+/*
+ * (C) Copyright 2011 Free Electrons
+ * David Wagner <david.wagner@free-electrons.com>
+ *
+ * Inspired from envcrc.c:
+ * (C) Copyright 2001
+ * Paolo Scaffardi, AIRVENT SAM s.p.a - RIMINI(ITALY), arsenio at tin.it
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+#include <unistd.h>
+#include <compiler.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#include <u-boot/crc.h>
+
+#define CRC_SIZE sizeof(uint32_t)
+
+static void usage(const char *exec_name)
+{
+	fprintf(stderr, "%s [-h] [-r] [-b] [-p <byte>] "
+	       "-s <environment partition size> -o <output> <input file>\n"
+	       "\n"
+	       "This tool takes a key=value input file (same as would a "
+	       "`printenv' show) and generates the corresponding environment "
+	       "image, ready to be flashed.\n"
+	       "\n"
+	       "\tThe input file is in format:\n"
+	       "\t\tkey1=value1\n"
+	       "\t\tkey2=value2\n"
+	       "\t\t...\n"
+	       "\t-r : the environment has multiple copies in flash\n"
+	       "\t-b : the target is big endian (default is little endian)\n"
+	       "\t-p <byte> : fill the image with <byte> bytes instead of "
+	       "0xff bytes\n"
+	       "\n"
+	       "If the input file is \"-\", data is read from standard input\n",
+	       exec_name);
+}
+
+int main(int argc, char **argv)
+{
+	uint32_t crc, targetendian_crc;
+	const char *txt_filename = NULL, *bin_filename = NULL;
+	int txt_fd, bin_fd;
+	unsigned char *dataptr, *envptr;
+	unsigned char *filebuf = NULL;
+	unsigned int filesize = 0, envsize = 0, datasize = 0;
+	int bigendian = 0;
+	int redundant = 0;
+	unsigned char padbyte = 0xff;
+
+	int option;
+	int ret = EXIT_SUCCESS;
+
+	struct stat txt_file_stat;
+
+	int fp, ep;
+
+	/* Parse the cmdline */
+	while ((option = getopt(argc, argv, "s:o:rbp:h")) != -1) {
+		switch (option) {
+		case 's':
+			datasize = strtol(optarg, NULL, 0);
+			break;
+		case 'o':
+			bin_filename = strdup(optarg);
+			if (!bin_filename) {
+				fprintf(stderr, "Can't strdup() the output "
+						"filename\n");
+				return EXIT_FAILURE;
+			}
+			break;
+		case 'r':
+			redundant = 1;
+			break;
+		case 'b':
+			bigendian = 1;
+			break;
+		case 'p':
+			padbyte = strtol(optarg, NULL, 0);
+			break;
+		case 'h':
+			usage(argv[0]);
+			return EXIT_SUCCESS;
+		default:
+			fprintf(stderr, "Wrong option -%c\n", option);
+			usage(argv[0]);
+			return EXIT_FAILURE;
+		}
+	}
+
+	/* Check datasize and allocate the data */
+	if (datasize == 0) {
+		fprintf(stderr,
+			"Please specify the size of the envrionnment "
+			"partition.\n");
+		usage(argv[0]);
+		return EXIT_FAILURE;
+	}
+
+	dataptr = malloc(datasize * sizeof(*dataptr));
+	if (!dataptr) {
+		fprintf(stderr, "Can't alloc dataptr.\n");
+		return EXIT_FAILURE;
+	}
+
+	/*
+	 * envptr points to the beginning of the actual environment (after the
+	 * crc and possible `redundant' bit
+	 */
+	envsize = datasize - (CRC_SIZE + redundant);
+	envptr = dataptr + CRC_SIZE + redundant;
+
+	/* Pad the environment with the padding byte */
+	memset(envptr, padbyte, envsize);
+
+	/* Open the input file ... */
+	if (optind >= argc) {
+		fprintf(stderr, "Please specify an input filename\n");
+		return EXIT_FAILURE;
+	}
+
+	txt_filename = argv[optind];
+	if (strcmp(txt_filename, "-") == 0) {
+		int readbytes = 0;
+		int readlen = sizeof(*envptr) * 2048;
+		txt_fd = STDIN_FILENO;
+
+		do {
+			filebuf = realloc(filebuf, readlen);
+			readbytes = read(txt_fd, filebuf + filesize, readlen);
+			filesize += readbytes;
+		} while (readbytes == readlen);
+
+	} else {
+		txt_fd = open(txt_filename, O_RDONLY);
+		if (txt_fd == -1) {
+			fprintf(stderr, "Can't open \"%s\": %s\n",
+					txt_filename, strerror(errno));
+			return EXIT_FAILURE;
+		}
+		/* ... and check it */
+		ret = fstat(txt_fd, &txt_file_stat);
+		if (ret == -1) {
+			fprintf(stderr, "Can't stat() on \"%s\": "
+					"%s\n", txt_filename, strerror(errno));
+			return EXIT_FAILURE;
+		}
+
+		filesize = txt_file_stat.st_size;
+		/* Read the raw input file and transform it */
+		filebuf = malloc(sizeof(*envptr) * filesize);
+		ret = read(txt_fd, filebuf, sizeof(*envptr) * filesize);
+		if (ret != sizeof(*envptr) * filesize) {
+			fprintf(stderr, "Can't read the whole input file\n");
+			return EXIT_FAILURE;
+		}
+		ret = close(txt_fd);
+	}
+	/*
+	 * The right test to do is "=>" (not ">") because of the additionnal
+	 * ending \0. See below.
+	 */
+	if (filesize >= envsize) {
+		fprintf(stderr, "The input file is larger than the "
+				"envrionnment partition size\n");
+		return EXIT_FAILURE;
+	}
+
+	/* Replace newlines separating variables with \0 */
+	for (fp = 0, ep = 0 ; fp < filesize ; fp++) {
+		if (filebuf[fp] == '\n') {
+			if (fp == 0) {
+				/*
+				 * Newline@the beggining of the file ?
+				 * Ignore it.
+				 */
+				continue;
+			} else if (filebuf[fp-1] == '\\') {
+				/*
+				 * Embedded newline in a variable.
+				 *
+				 * The backslash was added to the envptr ;
+				 * rewind and replace it with a newline
+				 */
+				ep--;
+				envptr[ep++] = '\n';
+			} else {
+				/* End of a variable */
+				envptr[ep++] = '\0';
+			}
+		} else if (filebuf[fp] == '#') {
+			if (fp != 0 && filebuf[fp-1] == '\n') {
+				/* This line is a comment, let's skip it */
+				while (fp < txt_file_stat.st_size &&
+				       filebuf[fp] != '\n')
+					fp++;
+			} else {
+				envptr[ep++] = filebuf[fp];
+			}
+		} else {
+			envptr[ep++] = filebuf[fp];
+		}
+	}
+	/*
+	 * Make sure there is a final '\0'
+	 * And do it again on the next byte to mark the end of the environment.
+	 */
+	if (envptr[ep-1] != '\0') {
+		envptr[ep++] = '\0';
+		/*
+		 * The text file doesn't have an ending newline.  We need to
+		 * check the env size again to make sure we have room for two \0
+		 */
+		if (ep >= envsize) {
+			fprintf(stderr, "The environment file is too large for "
+					"the target environment storage\n");
+			return EXIT_FAILURE;
+		}
+		envptr[ep] = '\0';
+	} else {
+		envptr[ep] = '\0';
+	}
+
+	/* Computes the CRC and put it at the beginning of the data */
+	crc = crc32(0, envptr, envsize);
+	targetendian_crc = bigendian ? cpu_to_be32(crc) : cpu_to_le32(crc);
+
+	memcpy(dataptr, &targetendian_crc, sizeof(uint32_t));
+
+	bin_fd = creat(bin_filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
+	if (bin_fd == -1) {
+		fprintf(stderr, "Can't open output file \"%s\": %s\n",
+				bin_filename, strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	if (write(bin_fd, dataptr, sizeof(*dataptr) * datasize) !=
+			sizeof(*dataptr) * datasize) {
+		fprintf(stderr, "write() failed: %s\n", strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	ret = close(bin_fd);
+
+	return ret;
+}
-- 
1.7.5.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv9] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-10-13 18:45 ` [U-Boot] [PATCHv9] " David Wagner
@ 2011-10-14  8:21   ` Detlev Zundel
  0 siblings, 0 replies; 31+ messages in thread
From: Detlev Zundel @ 2011-10-14  8:21 UTC (permalink / raw)
  To: u-boot

Hi David,

> This tool takes a key=value configuration file (same as would a `printenv' show)
> and generates the corresponding environment image, ready to be flashed.
>
> use case: flash the environment with an external tool
>
> Signed-off-by: David Wagner <david.wagner@free-electrons.com>
> Acked-by; Mike Frysinger <vapier@gentoo.org>

There's a semi-colon instead of a colon - this might make the tools
choke.

Cheers
  Detlev

-- 
"MS Windows User?"
"Yes..."
"Good. Line on the left, one cross each."
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv10] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-08-05 14:49 [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file David Wagner
                   ` (9 preceding siblings ...)
  2011-10-13 18:45 ` [U-Boot] [PATCHv9] " David Wagner
@ 2011-10-14 17:16 ` David Wagner
  2011-10-19  8:22   ` Thomas Petazzoni
                     ` (2 more replies)
  10 siblings, 3 replies; 31+ messages in thread
From: David Wagner @ 2011-10-14 17:16 UTC (permalink / raw)
  To: u-boot

This tool takes a key=value configuration file (same as would a `printenv' show)
and generates the corresponding environment image, ready to be flashed.

use case: flash the environment with an external tool

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
Acked-by: Mike Frysinger <vapier@gentoo.org>
Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
	change since v9:
	~~~~~~~~~~~~~~~~

 * fix a typo in the commit message spotted by Detlev Zundel

 tools/Makefile     |    5 +
 tools/mkenvimage.c |  272 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 277 insertions(+), 0 deletions(-)
 create mode 100644 tools/mkenvimage.c

diff --git a/tools/Makefile b/tools/Makefile
index fc741d3..da7caf0 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -66,6 +66,7 @@ BIN_FILES-$(CONFIG_BUILD_ENVCRC) += envcrc$(SFX)
 BIN_FILES-$(CONFIG_CMD_NET) += gen_eth_addr$(SFX)
 BIN_FILES-$(CONFIG_CMD_LOADS) += img2srec$(SFX)
 BIN_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes$(SFX)
+BIN_FILES-y += mkenvimage$(SFX)
 BIN_FILES-y += mkimage$(SFX)
 BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX)
 BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
@@ -89,6 +90,7 @@ OBJ_FILES-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes.o
 NOPED_OBJ_FILES-y += kwbimage.o
 NOPED_OBJ_FILES-y += imximage.o
 NOPED_OBJ_FILES-y += omapimage.o
+NOPED_OBJ_FILES-y += mkenvimage.o
 NOPED_OBJ_FILES-y += mkimage.o
 OBJ_FILES-$(CONFIG_NETCONSOLE) += ncb.o
 NOPED_OBJ_FILES-y += os_support.o
@@ -184,6 +186,9 @@ $(obj)xway-swap-bytes$(SFX):	$(obj)xway-swap-bytes.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
 	$(HOSTSTRIP) $@
 
+$(obj)mkenvimage$(SFX):	$(obj)crc32.o $(obj)mkenvimage.o
+	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
+
 $(obj)mkimage$(SFX):	$(obj)crc32.o \
 			$(obj)default_image.o \
 			$(obj)fit_image.o \
diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
new file mode 100644
index 0000000..2c7fbe7
--- /dev/null
+++ b/tools/mkenvimage.c
@@ -0,0 +1,272 @@
+/*
+ * (C) Copyright 2011 Free Electrons
+ * David Wagner <david.wagner@free-electrons.com>
+ *
+ * Inspired from envcrc.c:
+ * (C) Copyright 2001
+ * Paolo Scaffardi, AIRVENT SAM s.p.a - RIMINI(ITALY), arsenio at tin.it
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+#include <unistd.h>
+#include <compiler.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#include <u-boot/crc.h>
+
+#define CRC_SIZE sizeof(uint32_t)
+
+static void usage(const char *exec_name)
+{
+	fprintf(stderr, "%s [-h] [-r] [-b] [-p <byte>] "
+	       "-s <environment partition size> -o <output> <input file>\n"
+	       "\n"
+	       "This tool takes a key=value input file (same as would a "
+	       "`printenv' show) and generates the corresponding environment "
+	       "image, ready to be flashed.\n"
+	       "\n"
+	       "\tThe input file is in format:\n"
+	       "\t\tkey1=value1\n"
+	       "\t\tkey2=value2\n"
+	       "\t\t...\n"
+	       "\t-r : the environment has multiple copies in flash\n"
+	       "\t-b : the target is big endian (default is little endian)\n"
+	       "\t-p <byte> : fill the image with <byte> bytes instead of "
+	       "0xff bytes\n"
+	       "\n"
+	       "If the input file is \"-\", data is read from standard input\n",
+	       exec_name);
+}
+
+int main(int argc, char **argv)
+{
+	uint32_t crc, targetendian_crc;
+	const char *txt_filename = NULL, *bin_filename = NULL;
+	int txt_fd, bin_fd;
+	unsigned char *dataptr, *envptr;
+	unsigned char *filebuf = NULL;
+	unsigned int filesize = 0, envsize = 0, datasize = 0;
+	int bigendian = 0;
+	int redundant = 0;
+	unsigned char padbyte = 0xff;
+
+	int option;
+	int ret = EXIT_SUCCESS;
+
+	struct stat txt_file_stat;
+
+	int fp, ep;
+
+	/* Parse the cmdline */
+	while ((option = getopt(argc, argv, "s:o:rbp:h")) != -1) {
+		switch (option) {
+		case 's':
+			datasize = strtol(optarg, NULL, 0);
+			break;
+		case 'o':
+			bin_filename = strdup(optarg);
+			if (!bin_filename) {
+				fprintf(stderr, "Can't strdup() the output "
+						"filename\n");
+				return EXIT_FAILURE;
+			}
+			break;
+		case 'r':
+			redundant = 1;
+			break;
+		case 'b':
+			bigendian = 1;
+			break;
+		case 'p':
+			padbyte = strtol(optarg, NULL, 0);
+			break;
+		case 'h':
+			usage(argv[0]);
+			return EXIT_SUCCESS;
+		default:
+			fprintf(stderr, "Wrong option -%c\n", option);
+			usage(argv[0]);
+			return EXIT_FAILURE;
+		}
+	}
+
+	/* Check datasize and allocate the data */
+	if (datasize == 0) {
+		fprintf(stderr,
+			"Please specify the size of the envrionnment "
+			"partition.\n");
+		usage(argv[0]);
+		return EXIT_FAILURE;
+	}
+
+	dataptr = malloc(datasize * sizeof(*dataptr));
+	if (!dataptr) {
+		fprintf(stderr, "Can't alloc dataptr.\n");
+		return EXIT_FAILURE;
+	}
+
+	/*
+	 * envptr points to the beginning of the actual environment (after the
+	 * crc and possible `redundant' bit
+	 */
+	envsize = datasize - (CRC_SIZE + redundant);
+	envptr = dataptr + CRC_SIZE + redundant;
+
+	/* Pad the environment with the padding byte */
+	memset(envptr, padbyte, envsize);
+
+	/* Open the input file ... */
+	if (optind >= argc) {
+		fprintf(stderr, "Please specify an input filename\n");
+		return EXIT_FAILURE;
+	}
+
+	txt_filename = argv[optind];
+	if (strcmp(txt_filename, "-") == 0) {
+		int readbytes = 0;
+		int readlen = sizeof(*envptr) * 2048;
+		txt_fd = STDIN_FILENO;
+
+		do {
+			filebuf = realloc(filebuf, readlen);
+			readbytes = read(txt_fd, filebuf + filesize, readlen);
+			filesize += readbytes;
+		} while (readbytes == readlen);
+
+	} else {
+		txt_fd = open(txt_filename, O_RDONLY);
+		if (txt_fd == -1) {
+			fprintf(stderr, "Can't open \"%s\": %s\n",
+					txt_filename, strerror(errno));
+			return EXIT_FAILURE;
+		}
+		/* ... and check it */
+		ret = fstat(txt_fd, &txt_file_stat);
+		if (ret == -1) {
+			fprintf(stderr, "Can't stat() on \"%s\": "
+					"%s\n", txt_filename, strerror(errno));
+			return EXIT_FAILURE;
+		}
+
+		filesize = txt_file_stat.st_size;
+		/* Read the raw input file and transform it */
+		filebuf = malloc(sizeof(*envptr) * filesize);
+		ret = read(txt_fd, filebuf, sizeof(*envptr) * filesize);
+		if (ret != sizeof(*envptr) * filesize) {
+			fprintf(stderr, "Can't read the whole input file\n");
+			return EXIT_FAILURE;
+		}
+		ret = close(txt_fd);
+	}
+	/*
+	 * The right test to do is "=>" (not ">") because of the additionnal
+	 * ending \0. See below.
+	 */
+	if (filesize >= envsize) {
+		fprintf(stderr, "The input file is larger than the "
+				"envrionnment partition size\n");
+		return EXIT_FAILURE;
+	}
+
+	/* Replace newlines separating variables with \0 */
+	for (fp = 0, ep = 0 ; fp < filesize ; fp++) {
+		if (filebuf[fp] == '\n') {
+			if (fp == 0) {
+				/*
+				 * Newline@the beggining of the file ?
+				 * Ignore it.
+				 */
+				continue;
+			} else if (filebuf[fp-1] == '\\') {
+				/*
+				 * Embedded newline in a variable.
+				 *
+				 * The backslash was added to the envptr ;
+				 * rewind and replace it with a newline
+				 */
+				ep--;
+				envptr[ep++] = '\n';
+			} else {
+				/* End of a variable */
+				envptr[ep++] = '\0';
+			}
+		} else if (filebuf[fp] == '#') {
+			if (fp != 0 && filebuf[fp-1] == '\n') {
+				/* This line is a comment, let's skip it */
+				while (fp < txt_file_stat.st_size &&
+				       filebuf[fp] != '\n')
+					fp++;
+			} else {
+				envptr[ep++] = filebuf[fp];
+			}
+		} else {
+			envptr[ep++] = filebuf[fp];
+		}
+	}
+	/*
+	 * Make sure there is a final '\0'
+	 * And do it again on the next byte to mark the end of the environment.
+	 */
+	if (envptr[ep-1] != '\0') {
+		envptr[ep++] = '\0';
+		/*
+		 * The text file doesn't have an ending newline.  We need to
+		 * check the env size again to make sure we have room for two \0
+		 */
+		if (ep >= envsize) {
+			fprintf(stderr, "The environment file is too large for "
+					"the target environment storage\n");
+			return EXIT_FAILURE;
+		}
+		envptr[ep] = '\0';
+	} else {
+		envptr[ep] = '\0';
+	}
+
+	/* Computes the CRC and put it at the beginning of the data */
+	crc = crc32(0, envptr, envsize);
+	targetendian_crc = bigendian ? cpu_to_be32(crc) : cpu_to_le32(crc);
+
+	memcpy(dataptr, &targetendian_crc, sizeof(uint32_t));
+
+	bin_fd = creat(bin_filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
+	if (bin_fd == -1) {
+		fprintf(stderr, "Can't open output file \"%s\": %s\n",
+				bin_filename, strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	if (write(bin_fd, dataptr, sizeof(*dataptr) * datasize) !=
+			sizeof(*dataptr) * datasize) {
+		fprintf(stderr, "write() failed: %s\n", strerror(errno));
+		return EXIT_FAILURE;
+	}
+
+	ret = close(bin_fd);
+
+	return ret;
+}
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv10] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-10-14 17:16 ` [U-Boot] [PATCHv10] " David Wagner
@ 2011-10-19  8:22   ` Thomas Petazzoni
  2011-10-23 20:44   ` Wolfgang Denk
  2011-11-22  8:48   ` Stefano Babic
  2 siblings, 0 replies; 31+ messages in thread
From: Thomas Petazzoni @ 2011-10-19  8:22 UTC (permalink / raw)
  To: u-boot

Hello,

Le Fri, 14 Oct 2011 19:16:56 +0200,
David Wagner <david.wagner@free-electrons.com> a ?crit :

> This tool takes a key=value configuration file (same as would a
> `printenv' show) and generates the corresponding environment image,
> ready to be flashed.
> 
> use case: flash the environment with an external tool
> 
> Signed-off-by: David Wagner <david.wagner@free-electrons.com>
> Acked-by: Mike Frysinger <vapier@gentoo.org>
> Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Unless there are further comments, would it be possible to get this
merged for the upcoming U-Boot, or is the merge window already closed?

Thanks!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv10] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-10-14 17:16 ` [U-Boot] [PATCHv10] " David Wagner
  2011-10-19  8:22   ` Thomas Petazzoni
@ 2011-10-23 20:44   ` Wolfgang Denk
  2011-10-31  0:49     ` Mike Frysinger
  2011-11-22  8:48   ` Stefano Babic
  2 siblings, 1 reply; 31+ messages in thread
From: Wolfgang Denk @ 2011-10-23 20:44 UTC (permalink / raw)
  To: u-boot

Dear David Wagner,

In message <1318612616-16799-1-git-send-email-david.wagner@free-electrons.com> you wrote:
> This tool takes a key=value configuration file (same as would a `printenv' show)
> and generates the corresponding environment image, ready to be flashed.
> 
> use case: flash the environment with an external tool

This patch fails to build when I try to run a plain "make
tools/mkenvimage":

tools/mkenvimage.c:35:22: fatal error: compiler.h: No such file or directory

tools/mkenvimage.c:39:24: fatal error: u-boot/crc.h: No such file or
directory



...
> +	/* Parse the cmdline */
> +	while ((option = getopt(argc, argv, "s:o:rbp:h")) != -1) {
> +		switch (option) {
> +		case 's':
> +			datasize = strtol(optarg, NULL, 0);
...
> +			padbyte = strtol(optarg, NULL, 0);

Please add error checking for invalid input formats here.

> +	if (datasize == 0) {
> +		fprintf(stderr,
> +			"Please specify the size of the envrionnment "
> +			"partition.\n");

Please don't split that string, and fix the "envrionment" typo.

> +	dataptr = malloc(datasize * sizeof(*dataptr));
> +	if (!dataptr) {
> +		fprintf(stderr, "Can't alloc dataptr.\n");

It might be helpful to know how many bytes you were trying to
allocate.

> +	/*
> +	 * envptr points to the beginning of the actual environment (after the
> +	 * crc and possible `redundant' bit

s/bit/byte/

> +	/* Open the input file ... */
> +	if (optind >= argc) {
> +		fprintf(stderr, "Please specify an input filename\n");
> +		return EXIT_FAILURE;
> +	}

Please also allow to use stdin as input when no "<input file>" arg is
given.

> +		int readlen = sizeof(*envptr) * 2048;
> +		txt_fd = STDIN_FILENO;
> +
> +		do {
> +			filebuf = realloc(filebuf, readlen);
> +			readbytes = read(txt_fd, filebuf + filesize, readlen);
> +			filesize += readbytes;
> +		} while (readbytes == readlen);

This is pretty much inefficient.  Consider a size increment of
something like  min(readlen, 4096).

Also, realloc() can fail - add error handling.

And read() can fail, too - add error handling.

> +		filesize = txt_file_stat.st_size;
> +		/* Read the raw input file and transform it */

Why don't you just use mmap() here?

> +	if (filesize >= envsize) {
> +		fprintf(stderr, "The input file is larger than the "
> +				"envrionnment partition size\n");

Please don't split such strings.  See CodingStyle:

        "However, never break user-visible strings such as printk
         messages, because that breaks the ability to grep for them."

Please fix globally.

> +		} else if (filebuf[fp] == '#') {
> +			if (fp != 0 && filebuf[fp-1] == '\n') {
> +				/* This line is a comment, let's skip it */
> +				while (fp < txt_file_stat.st_size &&
> +				       filebuf[fp] != '\n')
> +					fp++;
> +			} else {
> +				envptr[ep++] = filebuf[fp];
> +			}

printenv output does not contain any such "comments".
And - aren't you also catching embedded hashes here, like in "serial#"
for example?

...
> +
> +	/* Computes the CRC and put it at the beginning of the data */
> +	crc = crc32(0, envptr, envsize);
> +	targetendian_crc = bigendian ? cpu_to_be32(crc) : cpu_to_le32(crc);
> +
> +	memcpy(dataptr, &targetendian_crc, sizeof(uint32_t));

I fail to see where you set the redundant flag?


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I am more bored than you could ever possibly be.  Go back to work.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv10] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-10-23 20:44   ` Wolfgang Denk
@ 2011-10-31  0:49     ` Mike Frysinger
  0 siblings, 0 replies; 31+ messages in thread
From: Mike Frysinger @ 2011-10-31  0:49 UTC (permalink / raw)
  To: u-boot

On Sunday 23 October 2011 16:44:38 Wolfgang Denk wrote:
> David Wagner wrote:
> > This tool takes a key=value configuration file (same as would a
> > `printenv' show) and generates the corresponding environment image,
> > ready to be flashed.
> > 
> > use case: flash the environment with an external tool
> 
> This patch fails to build when I try to run a plain "make
> tools/mkenvimage":
> 
> tools/mkenvimage.c:35:22: fatal error: compiler.h: No such file or
> directory
> tools/mkenvimage.c:39:24: fatal error: u-boot/crc.h: No such file or
> directory

i think that's expected.  if you do `make tools`, does it build correctly ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111030/3fb80ddb/attachment.pgp 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [PATCHv10] new tool mkenvimage: generates an env image from an arbitrary config file
  2011-10-14 17:16 ` [U-Boot] [PATCHv10] " David Wagner
  2011-10-19  8:22   ` Thomas Petazzoni
  2011-10-23 20:44   ` Wolfgang Denk
@ 2011-11-22  8:48   ` Stefano Babic
  2 siblings, 0 replies; 31+ messages in thread
From: Stefano Babic @ 2011-11-22  8:48 UTC (permalink / raw)
  To: u-boot

On 14/10/2011 19:16, David Wagner wrote:
> This tool takes a key=value configuration file (same as would a `printenv' show)
> and generates the corresponding environment image, ready to be flashed.
> 
> use case: flash the environment with an external tool
> 
> Signed-off-by: David Wagner <david.wagner@free-electrons.com>
> Acked-by: Mike Frysinger <vapier@gentoo.org>
> Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Applied to u-boot-staging, sbabic at denx.de, thanks.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2011-11-22  8:48 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-05 14:49 [U-Boot] [PATCH] new tool mkenvimage: generates an env image from an arbitrary config file David Wagner
2011-08-05 15:23 ` Thomas Petazzoni
2011-08-05 16:23 ` [U-Boot] [PATCHv2] " David Wagner
2011-08-06 11:18   ` Mike Frysinger
2011-08-08  8:16     ` David Wagner
2011-08-08  8:46       ` Albert ARIBAUD
2011-08-08  8:56         ` David Wagner
2011-08-08 18:53       ` Mike Frysinger
2011-08-09 10:31 ` [U-Boot] [PATCH] " David Wagner
2011-08-22  1:09   ` Mike Frysinger
2011-08-24 22:12   ` Wolfgang Denk
2011-08-29 12:06 ` [U-Boot] [PATCHv4] " David Wagner
2011-08-30 19:12   ` Mike Frysinger
2011-08-30 19:34   ` Wolfgang Denk
2011-09-01 15:57 ` [U-Boot] [PATCHv5] " David Wagner
2011-09-01 17:47   ` Mike Frysinger
2011-09-02  8:47 ` [U-Boot] [PATCH] " David Wagner
2011-09-02  8:48   ` David Wagner
2011-09-02  8:48 ` [U-Boot] [PATCHv6] " David Wagner
2011-09-02 14:35   ` Mike Frysinger
2011-09-26 12:10   ` Thomas Petazzoni
2011-09-26 13:26 ` [U-Boot] [PATCHv7] " David Wagner
2011-09-30  7:31 ` [U-Boot] [PATCHv8] " David Wagner
2011-10-06 21:17   ` Wolfgang Denk
2011-10-13 18:45 ` [U-Boot] [PATCHv9] " David Wagner
2011-10-14  8:21   ` Detlev Zundel
2011-10-14 17:16 ` [U-Boot] [PATCHv10] " David Wagner
2011-10-19  8:22   ` Thomas Petazzoni
2011-10-23 20:44   ` Wolfgang Denk
2011-10-31  0:49     ` Mike Frysinger
2011-11-22  8:48   ` Stefano Babic

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.