[U-Boot] [PATCH 0/9] mkenvimage: v10/v11 diff split into several patches

[U-Boot] [PATCH 0/9] mkenvimage: v10/v11 diff split into several patches

[David Wagner]

Hi,

First, thanks for having mainlined mkenvimage :)

These patches mainly address Wolfgang's remarks.  There are a couple others.

I first intended to submit a v11 patch but since the v10 was merged into
u-boot's master branch, I splitted the changes into 9 patches against it.


Regards,
	David.

David Wagner (9):
  Strip mkenvimage
  mkenvimage: correct and clarify comments and error messages
  mkenvimage: Correct the includes and add a missing one
  mkenvimage: More error handling
  mkenvimage: Read from stdin if the filename is "-"
  mkenvimage: Use mmap() when reading from a regular file
  mkenvimage: Don't try to detect comments in the input file
  mkenvimage: Really set the redundant byte when applicable
  mkenvimage: Default to stdout if the output argument is absent or "-"

 tools/Makefile     |    1 +
 tools/mkenvimage.c |  128 +++++++++++++++++++++++++++++-----------------------
 2 files changed, 72 insertions(+), 57 deletions(-)

-- 
1.7.7.3

[U-Boot] [PATCH 1/9] Strip mkenvimage

[David Wagner]

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/Makefile b/tools/Makefile
index a5f989a..64bcc4d 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -194,6 +194,7 @@ $(obj)xway-swap-bytes$(SFX):	$(obj)xway-swap-bytes.o
 
 $(obj)mkenvimage$(SFX):	$(obj)crc32.o $(obj)mkenvimage.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
+	$(HOSTSTRIP) $@
 
 $(obj)mkimage$(SFX):	$(obj)aisimage.o \
 			$(obj)crc32.o \
-- 
1.7.7.3

[U-Boot] [PATCH 2/9] mkenvimage: correct and clarify comments and error messages

[David Wagner]

Also, don't split error messages over several lines as per a coding style
exception making them easier to grep.

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |   39 ++++++++++++++-------------------------
 1 files changed, 14 insertions(+), 25 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index 9c32f4a..da54658 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -41,12 +41,9 @@
 
 static void usage(const char *exec_name)
 {
-	fprintf(stderr, "%s [-h] [-r] [-b] [-p <byte>] "
-	       "-s <environment partition size> -o <output> <input file>\n"
+	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"
+	       "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"
@@ -54,8 +51,7 @@ static void usage(const char *exec_name)
 	       "\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"
+	       "\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);
@@ -89,8 +85,7 @@ int main(int argc, char **argv)
 		case 'o':
 			bin_filename = strdup(optarg);
 			if (!bin_filename) {
-				fprintf(stderr, "Can't strdup() the output "
-						"filename\n");
+				fprintf(stderr, "Can't strdup() the output filename\n");
 				return EXIT_FAILURE;
 			}
 			break;
@@ -115,22 +110,21 @@ int main(int argc, char **argv)
 
 	/* Check datasize and allocate the data */
 	if (datasize == 0) {
-		fprintf(stderr,
-			"Please specify the size of the envrionnment "
-			"partition.\n");
+		fprintf(stderr, "Please specify the size of the environment partition.\n");
 		usage(argv[0]);
 		return EXIT_FAILURE;
 	}
 
 	dataptr = malloc(datasize * sizeof(*dataptr));
 	if (!dataptr) {
-		fprintf(stderr, "Can't alloc dataptr.\n");
+		fprintf(stderr, "Can't alloc %d bytes for dataptr.\n",
+				datasize);
 		return EXIT_FAILURE;
 	}
 
 	/*
 	 * envptr points to the beginning of the actual environment (after the
-	 * crc and possible `redundant' bit
+	 * crc and possible `redundant' byte
 	 */
 	envsize = datasize - (CRC_SIZE + redundant);
 	envptr = dataptr + CRC_SIZE + redundant;
@@ -166,8 +160,8 @@ int main(int argc, char **argv)
 		/* ... 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));
+			fprintf(stderr, "Can't stat() on \"%s\": %s\n",
+					txt_filename, strerror(errno));
 			return EXIT_FAILURE;
 		}
 
@@ -181,13 +175,9 @@ int main(int argc, char **argv)
 		}
 		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");
+	/* The +1 is for the additionnal ending \0. See below. */
+	if (filesize + 1 > envsize) {
+		fprintf(stderr, "The input file is larger than the environment partition size\n");
 		return EXIT_FAILURE;
 	}
 
@@ -236,8 +226,7 @@ int main(int argc, char **argv)
 		 * 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");
+			fprintf(stderr, "The environment file is too large for the target environment storage\n");
 			return EXIT_FAILURE;
 		}
 		envptr[ep] = '\0';
-- 
1.7.7.3

[U-Boot] [PATCH 3/9] mkenvimage: Correct the includes and add a missing one

[David Wagner]

compiler.h and u-boot/crc.h need to be included from U-Boot's headers.

stdlib.h was missing.

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index da54658..6af0d56 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -28,14 +28,15 @@
 #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 "compiler.h"
 #include <sys/types.h>
 #include <sys/stat.h>
 
-#include <u-boot/crc.h>
+#include "u-boot/crc.h"
 
 #define CRC_SIZE sizeof(uint32_t)
 
-- 
1.7.7.3

[U-Boot] [PATCH 4/9] mkenvimage: More error handling

[David Wagner]

Verbosly fail if the target environment size or the padding byte are badly
formated.

Verbosly fail if something bad happens when reading from standard input.

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |   32 ++++++++++++++++++++++++++++++--
 1 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index 6af0d56..b6e7f14 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -82,7 +82,17 @@ int main(int argc, char **argv)
 		switch (option) {
 		case 's':
 			datasize = strtol(optarg, NULL, 0);
-			break;
+			if (!errno)
+				break;
+
+			if (errno == ERANGE)
+				fprintf(stderr, "Bad integer format: %s\n",
+						optarg);
+			else
+				fprintf(stderr, "Error while parsing %s: %s\n",
+						optarg, strerror(errno));
+
+			return EXIT_FAILURE;
 		case 'o':
 			bin_filename = strdup(optarg);
 			if (!bin_filename) {
@@ -98,7 +108,17 @@ int main(int argc, char **argv)
 			break;
 		case 'p':
 			padbyte = strtol(optarg, NULL, 0);
-			break;
+			if (!errno)
+				break;
+
+			if (errno == ERANGE)
+				fprintf(stderr, "Bad integer format: %s\n",
+						optarg);
+			else
+				fprintf(stderr, "Error while parsing %s: %s\n",
+						optarg, strerror(errno));
+
+			return EXIT_FAILURE;
 		case 'h':
 			usage(argv[0]);
 			return EXIT_SUCCESS;
@@ -147,7 +167,15 @@ int main(int argc, char **argv)
 
 		do {
 			filebuf = realloc(filebuf, readlen);
+			if (!filebuf) {
+				fprintf(stderr, "Can't realloc memory for the input file buffer\n");
+				return EXIT_FAILURE;
+			}
 			readbytes = read(txt_fd, filebuf + filesize, readlen);
+			if (errno) {
+				fprintf(stderr, "Error while reading stdin: %s\n", strerror(errno));
+				return EXIT_FAILURE;
+			}
 			filesize += readbytes;
 		} while (readbytes == readlen);
 
-- 
1.7.7.3

[U-Boot] [PATCH 5/9] mkenvimage: Read from stdin if the filename is "-"

[David Wagner]

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index b6e7f14..86a4e05 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -154,15 +154,9 @@ int main(int argc, char **argv)
 	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) {
+	if (optind >= argc || strcmp(argv[optind], "-") == 0) {
 		int readbytes = 0;
-		int readlen = sizeof(*envptr) * 2048;
+		int readlen = sizeof(*envptr) * 4096;
 		txt_fd = STDIN_FILENO;
 
 		do {
@@ -180,6 +174,7 @@ int main(int argc, char **argv)
 		} while (readbytes == readlen);
 
 	} else {
+		txt_filename = argv[optind];
 		txt_fd = open(txt_filename, O_RDONLY);
 		if (txt_fd == -1) {
 			fprintf(stderr, "Can't open \"%s\": %s\n",
-- 
1.7.7.3

[U-Boot] [PATCH 6/9] mkenvimage: Use mmap() when reading from a regular file

[David Wagner]

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index 86a4e05..74b296e 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -35,6 +35,7 @@
 #include "compiler.h"
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/mman.h>
 
 #include "u-boot/crc.h"
 
@@ -190,14 +191,16 @@ int main(int argc, char **argv)
 		}
 
 		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");
+
+		filebuf = mmap(NULL, sizeof(*envptr) * filesize, PROT_READ,
+			       MAP_PRIVATE, txt_fd, 0);
+		ret = close(txt_fd);
+		if (filebuf == MAP_FAILED) {
+			fprintf(stderr, "mmap (%d bytes) failed: %s\n",
+					sizeof(*envptr) * filesize,
+					strerror(errno));
 			return EXIT_FAILURE;
 		}
-		ret = close(txt_fd);
 	}
 	/* The +1 is for the additionnal ending \0. See below. */
 	if (filesize + 1 > envsize) {
-- 
1.7.7.3

[U-Boot] [PATCH 7/9] mkenvimage: Don't try to detect comments in the input file

[David Wagner]

Remove this feature since it seems impossible to reliably detect them.

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index 74b296e..8d84c85 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -230,14 +230,6 @@ int main(int argc, char **argv)
 				/* 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];
 		}
-- 
1.7.7.3

[U-Boot] [PATCH 8/9] mkenvimage: Really set the redundant byte when applicable

[David Wagner]

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index 8d84c85..fa6eb6e 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -257,7 +257,9 @@ int main(int argc, char **argv)
 	crc = crc32(0, envptr, envsize);
 	targetendian_crc = bigendian ? cpu_to_be32(crc) : cpu_to_le32(crc);
 
-	memcpy(dataptr, &targetendian_crc, sizeof(uint32_t));
+	memcpy(dataptr, &targetendian_crc, sizeof(targetendian_crc));
+	if (redundant)
+		*(dataptr + sizeof(targetendian_crc)) = 1;
 
 	bin_fd = creat(bin_filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
 	if (bin_fd == -1) {
-- 
1.7.7.3

[U-Boot] [PATCH 9/9] mkenvimage: Default to stdout if the output argument is absent or "-"

[David Wagner]

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index fa6eb6e..8ba63d2 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -261,11 +261,15 @@ int main(int argc, char **argv)
 	if (redundant)
 		*(dataptr + sizeof(targetendian_crc)) = 1;
 
-	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 (!bin_filename || strcmp(bin_filename, "-") == 0) {
+		bin_fd = STDOUT_FILENO;
+	} else {
+		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) !=
-- 
1.7.7.3

[U-Boot] [PATCH 8/9] mkenvimage: Really set the redundant byte when applicable

[Mike Frysinger]

On Wednesday 23 November 2011 15:28:17 David Wagner wrote:
> --- a/tools/mkenvimage.c
> +++ b/tools/mkenvimage.c
> 
> +	if (redundant)
> +		*(dataptr + sizeof(targetendian_crc)) = 1;

dataptr[sizeof(targetendian_crc)] = 1;
-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/20111125/d9f70595/attachment.pgp>

[U-Boot] [PATCH 9/9] mkenvimage: Default to stdout if the output argument is absent or "-"

[Mike Frysinger]

squash this into the first stdin patch ?
[PATCH 5/9] mkenvimage: Read from stdin if the filename is "-"
-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/20111125/a9bcfac0/attachment.pgp>

[U-Boot] [PATCHv2 5/8] mkenvimage: Read/Write from/to stdin/out by default or if the filename is "-"

[David Wagner]

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

This 2nd version is a meld of the 1st version and patch 9

 tools/mkenvimage.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index b6e7f14..ab67c1a 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -154,15 +154,9 @@ int main(int argc, char **argv)
 	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) {
+	if (optind >= argc || strcmp(argv[optind], "-") == 0) {
 		int readbytes = 0;
-		int readlen = sizeof(*envptr) * 2048;
+		int readlen = sizeof(*envptr) * 4096;
 		txt_fd = STDIN_FILENO;
 
 		do {
@@ -180,6 +174,7 @@ int main(int argc, char **argv)
 		} while (readbytes == readlen);
 
 	} else {
+		txt_filename = argv[optind];
 		txt_fd = open(txt_filename, O_RDONLY);
 		if (txt_fd == -1) {
 			fprintf(stderr, "Can't open \"%s\": %s\n",
@@ -269,11 +264,16 @@ int main(int argc, char **argv)
 
 	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 (!bin_filename || strcmp(bin_filename, "-") == 0) {
+		bin_fd = STDOUT_FILENO;
+	} else {
+		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) !=
-- 
1.7.7.3

[U-Boot] [PATCHv2 8/8] mkenvimage: Really set the redundant byte when applicable

[David Wagner]

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index 221e9de..4e69056 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -257,7 +257,9 @@ int main(int argc, char **argv)
 	crc = crc32(0, envptr, envsize);
 	targetendian_crc = bigendian ? cpu_to_be32(crc) : cpu_to_le32(crc);
 
-	memcpy(dataptr, &targetendian_crc, sizeof(uint32_t));
+	memcpy(dataptr, &targetendian_crc, sizeof(targetendian_crc));
+	if (redundant)
+		dataptr[sizeof(targetendian_crc)] = 1;
 
 	if (!bin_filename || strcmp(bin_filename, "-") == 0) {
 		bin_fd = STDOUT_FILENO;
-- 
1.7.7.3

[U-Boot] [PATCH 9/9] mkenvimage: Default to stdout if the output argument is absent or "-"

[David Wagner]

On 25/11/2011 22:36, Mike Frysinger wrote:
> squash this into the first stdin patch ?
> [PATCH 5/9] mkenvimage: Read from stdin if the filename is "-"
> -mike

Done, you can ignore this patch.

David.

[U-Boot] [PATCH 0/9] mkenvimage: v10/v11 diff split into several patches

[Mike Frysinger]

don't see anything wrong with the latest patchset now, 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/20111127/f8521a6b/attachment.pgp>

[U-Boot] [PATCHv2 02/10] mkenvimage: correct and clarify comments and error messages

[David Wagner]

Also, don't split error messages over several lines as per a coding style
exception making them easier to grep.

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

This version is rebased on top of 'next' (it didn't apply anymore, after Horst
Kronstorfer's patches).

 tools/mkenvimage.c |   35 +++++++++++++++++------------------
 1 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index 753d9e6..5f7d6ea 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -42,12 +42,9 @@
 
 static void usage(const char *exec_name)
 {
-	fprintf(stderr, "%s [-h] [-r] [-b] [-p <byte>] "
-	       "-s <environment partition size> -o <output> <input file>\n"
+	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"
+	       "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"
@@ -55,8 +52,7 @@ static void usage(const char *exec_name)
 	       "\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"
+	       "\t-p <byte> : fill the image with <byte> bytes instead of 0xff bytes\n"
 	       "\t-V : print version information and exit\n"
 	       "\n"
 	       "If the input file is \"-\", data is read from standard input\n",
@@ -94,8 +90,7 @@ int main(int argc, char **argv)
 		case 'o':
 			bin_filename = strdup(optarg);
 			if (!bin_filename) {
-				fprintf(stderr, "Can't strdup() the output "
-						"filename\n");
+				fprintf(stderr, "Can't strdup() the output filename\n");
 				return EXIT_FAILURE;
 			}
 			break;
@@ -128,22 +123,21 @@ int main(int argc, char **argv)
 
 	/* Check datasize and allocate the data */
 	if (datasize == 0) {
-		fprintf(stderr,
-			"Please specify the size of the environment "
-			"partition.\n");
+		fprintf(stderr, "Please specify the size of the environment partition.\n");
 		usage(argv[0]);
 		return EXIT_FAILURE;
 	}
 
 	dataptr = malloc(datasize * sizeof(*dataptr));
 	if (!dataptr) {
-		fprintf(stderr, "Can't alloc dataptr.\n");
+		fprintf(stderr, "Can't alloc %d bytes for dataptr.\n",
+				datasize);
 		return EXIT_FAILURE;
 	}
 
 	/*
 	 * envptr points to the beginning of the actual environment (after the
-	 * crc and possible `redundant' bit
+	 * crc and possible `redundant' byte
 	 */
 	envsize = datasize - (CRC_SIZE + redundant);
 	envptr = dataptr + CRC_SIZE + redundant;
@@ -179,8 +173,8 @@ int main(int argc, char **argv)
 		/* ... 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));
+			fprintf(stderr, "Can't stat() on \"%s\": %s\n",
+					txt_filename, strerror(errno));
 			return EXIT_FAILURE;
 		}
 
@@ -194,6 +188,7 @@ int main(int argc, char **argv)
 		}
 		ret = close(txt_fd);
 	}
+<<<<<<< HEAD
 	/*
 	 * The right test to do is "=>" (not ">") because of the additional
 	 * ending \0. See below.
@@ -201,6 +196,11 @@ int main(int argc, char **argv)
 	if (filesize >= envsize) {
 		fprintf(stderr, "The input file is larger than the "
 				"environment partition size\n");
+=======
+	/* The +1 is for the additionnal ending \0. See below. */
+	if (filesize + 1 > envsize) {
+		fprintf(stderr, "The input file is larger than the environment partition size\n");
+>>>>>>> mkenvimage: correct and clarify comments and error messages
 		return EXIT_FAILURE;
 	}
 
@@ -249,8 +249,7 @@ int main(int argc, char **argv)
 		 * 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");
+			fprintf(stderr, "The environment file is too large for the target environment storage\n");
 			return EXIT_FAILURE;
 		}
 		envptr[ep] = '\0';
-- 
1.7.5.4

[U-Boot] [PATCHv2 03/10] mkenvimage: Correct the includes and add a missing one

[David Wagner]

compiler.h and u-boot/crc.h need to be included from U-Boot's headers.

stdlib.h was missing.

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

This version is rebased on top of 'next' (it didn't apply anymore, after Horst
Kronstorfer's patches).

 tools/mkenvimage.c |   15 +++------------
 1 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index 5f7d6ea..b8b5c3a 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -28,14 +28,15 @@
 #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 "compiler.h"
 #include <sys/types.h>
 #include <sys/stat.h>
 
-#include <u-boot/crc.h>
+#include "u-boot/crc.h"
 #include <version.h>
 
 #define CRC_SIZE sizeof(uint32_t)
@@ -188,19 +189,9 @@ int main(int argc, char **argv)
 		}
 		ret = close(txt_fd);
 	}
-<<<<<<< HEAD
-	/*
-	 * The right test to do is "=>" (not ">") because of the additional
-	 * ending \0. See below.
-	 */
-	if (filesize >= envsize) {
-		fprintf(stderr, "The input file is larger than the "
-				"environment partition size\n");
-=======
 	/* The +1 is for the additionnal ending \0. See below. */
 	if (filesize + 1 > envsize) {
 		fprintf(stderr, "The input file is larger than the environment partition size\n");
->>>>>>> mkenvimage: correct and clarify comments and error messages
 		return EXIT_FAILURE;
 	}
 
-- 
1.7.5.4

[U-Boot] [PATCH 0/9] mkenvimage: v10/v11 diff split into several patches

[David Wagner]

Le 27/11/2011 20:59, Mike Frysinger a ?crit :
> don't see anything wrong with the latest patchset now, thanks!
> -mike

Hi,

I re-sent 2 patches that didn't apply anymore on top of next (v2 2/10 
and v2 3/10) ; the others should be fine.

I also sent one more patch ([PATCH 09/10] Correctly handle input files 
beginning with several newlines)

Could you please apply them ? I have one more patch in line that adds 
Cygwin support, still to be tested.


Thanks!

	David.

[U-Boot] [PATCHv2 02/10] mkenvimage: correct and clarify comments and error messages

[Thomas Petazzoni]

Le Wed, 21 Dec 2011 01:58:25 +0100,
David Wagner <david.wagner@free-electrons.com> a ?crit :

> +<<<<<<< HEAD
>  	/*
>  	 * The right test to do is "=>" (not ">") because of the additional
>  	 * ending \0. See below.
> @@ -201,6 +196,11 @@ int main(int argc, char **argv)
>  	if (filesize >= envsize) {
>  		fprintf(stderr, "The input file is larger than the "
>  				"environment partition size\n");
> +=======
> +	/* The +1 is for the additionnal ending \0. See below. */
> +	if (filesize + 1 > envsize) {
> +		fprintf(stderr, "The input file is larger than the environment partition size\n");
> +>>>>>>> mkenvimage: correct and clarify comments and error messages

Seems like your forgot to resolve some conflicts. A compile test would
have detected those issues.

Regards,

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

[U-Boot] [PATCH 0/9] mkenvimage: v10/v11 diff split into several patches

[Stefano Babic]

On 21/12/2011 02:04, David Wagner wrote:
> Hi,
> 
> I re-sent 2 patches that didn't apply anymore on top of next (v2 2/10
> and v2 3/10) ; the others should be fine.
> 
> I also sent one more patch ([PATCH 09/10] Correctly handle input files
> beginning with several newlines)
> 
> Could you please apply them ? 

See Thomas's answer : there are not resolved conflicts in your last
patch. Solve them, and I will take care of the patchset. However, I do
not know if it can go into the release (Wolfgang will decide about it).

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
=====================================================================

[U-Boot] [PATCHv2 02/10] mkenvimage: correct and clarify comments and error messages

[David Wagner]

Le 21/12/2011 08:41, Thomas Petazzoni a ?crit :
> Seems like your forgot to resolve some conflicts. A compile test would
> have detected those issues.
>
> Regards,
>
> Thomas

Ow. right...

However, I will first wait for Horst's patch 3/4 (print program basename 
instead of whole path in usage()) to be applied because even after 
correcting my mistake, it won't compile (patch 4/4, which have been 
applied on 'next', depends on 3/4).

Regards,
David.

[U-Boot] [PATCHv2 03/10] mkenvimage: Correct the includes and add a missing one

[Wolfgang Denk]

Dear David Wagner,

In message <1324429120-10141-1-git-send-email-david.wagner@free-electrons.com> you wrote:
> compiler.h and u-boot/crc.h need to be included from U-Boot's headers.
> 
> stdlib.h was missing.
> 
> Signed-off-by: David Wagner <david.wagner@free-electrons.com>
> ---
> 
> This version is rebased on top of 'next' (it didn't apply anymore, after Horst
> Kronstorfer's patches).

Please rebase against current master 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
"There are things that are so serious that you can  only  joke  about
them"                                                    - Heisenberg

[U-Boot] [PATCHv2 1/8] Strip mkenvimage

[David Wagner]

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/Makefile b/tools/Makefile
index a5f989a..64bcc4d 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -194,6 +194,7 @@ $(obj)xway-swap-bytes$(SFX):	$(obj)xway-swap-bytes.o
 
 $(obj)mkenvimage$(SFX):	$(obj)crc32.o $(obj)mkenvimage.o
 	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^
+	$(HOSTSTRIP) $@
 
 $(obj)mkimage$(SFX):	$(obj)aisimage.o \
 			$(obj)crc32.o \
-- 
1.7.5.4

[U-Boot] [PATCHv3 2/8] mkenvimage: correct and clarify comments and error messages

[David Wagner]

Also, don't split error messages over several lines as per a coding style
exception making them easier to grep.

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |   41 +++++++++++++++--------------------------
 1 files changed, 15 insertions(+), 26 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index c5ed373..7d33143 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -45,12 +45,9 @@
 
 static void usage(const char *exec_name)
 {
-	fprintf(stderr, "%s [-h] [-r] [-b] [-p <byte>] "
-	       "-s <environment partition size> -o <output> <input file>\n"
+	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"
+	       "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"
@@ -58,8 +55,7 @@ static void usage(const char *exec_name)
 	       "\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"
+	       "\t-p <byte> : fill the image with <byte> bytes instead of 0xff bytes\n"
 	       "\t-V : print version information and exit\n"
 	       "\n"
 	       "If the input file is \"-\", data is read from standard input\n",
@@ -100,8 +96,7 @@ int main(int argc, char **argv)
 		case 'o':
 			bin_filename = strdup(optarg);
 			if (!bin_filename) {
-				fprintf(stderr, "Can't strdup() the output "
-						"filename\n");
+				fprintf(stderr, "Can't strdup() the output filename\n");
 				return EXIT_FAILURE;
 			}
 			break;
@@ -118,7 +113,7 @@ int main(int argc, char **argv)
 			usage(prg);
 			return EXIT_SUCCESS;
 		case 'V':
-			printf("%s version %s\n", prg, PLAIN_VERSION);
+			printf("%s version %s\n", argv[0], PLAIN_VERSION);
 			return EXIT_SUCCESS;
 		case ':':
 			fprintf(stderr, "Missing argument for option -%c\n",
@@ -134,22 +129,21 @@ int main(int argc, char **argv)
 
 	/* Check datasize and allocate the data */
 	if (datasize == 0) {
-		fprintf(stderr,
-			"Please specify the size of the environment "
-			"partition.\n");
+		fprintf(stderr, "Please specify the size of the environment partition.\n");
 		usage(prg);
 		return EXIT_FAILURE;
 	}
 
 	dataptr = malloc(datasize * sizeof(*dataptr));
 	if (!dataptr) {
-		fprintf(stderr, "Can't alloc dataptr.\n");
+		fprintf(stderr, "Can't alloc %d bytes for dataptr.\n",
+				datasize);
 		return EXIT_FAILURE;
 	}
 
 	/*
 	 * envptr points to the beginning of the actual environment (after the
-	 * crc and possible `redundant' bit
+	 * crc and possible `redundant' byte
 	 */
 	envsize = datasize - (CRC_SIZE + redundant);
 	envptr = dataptr + CRC_SIZE + redundant;
@@ -185,8 +179,8 @@ int main(int argc, char **argv)
 		/* ... 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));
+			fprintf(stderr, "Can't stat() on \"%s\": %s\n",
+					txt_filename, strerror(errno));
 			return EXIT_FAILURE;
 		}
 
@@ -200,13 +194,9 @@ int main(int argc, char **argv)
 		}
 		ret = close(txt_fd);
 	}
-	/*
-	 * The right test to do is "=>" (not ">") because of the additional
-	 * ending \0. See below.
-	 */
-	if (filesize >= envsize) {
-		fprintf(stderr, "The input file is larger than the "
-				"environment partition size\n");
+	/* The +1 is for the additionnal ending \0. See below. */
+	if (filesize + 1 > envsize) {
+		fprintf(stderr, "The input file is larger than the environment partition size\n");
 		return EXIT_FAILURE;
 	}
 
@@ -255,8 +245,7 @@ int main(int argc, char **argv)
 		 * 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");
+			fprintf(stderr, "The environment file is too large for the target environment storage\n");
 			return EXIT_FAILURE;
 		}
 		envptr[ep] = '\0';
-- 
1.7.5.4

[U-Boot] [PATCHv3 3/8] mkenvimage: Correct the includes and add a missing one

[David Wagner]

compiler.h and u-boot/crc.h need to be included from U-Boot's headers.

stdlib.h was missing.

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index 7d33143..bc18736 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -31,14 +31,15 @@
 #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 "compiler.h"
 #include <sys/types.h>
 #include <sys/stat.h>
 
-#include <u-boot/crc.h>
+#include "u-boot/crc.h"
 #include <version.h>
 
 #define CRC_SIZE sizeof(uint32_t)
-- 
1.7.5.4

[U-Boot] [PATCHv2 4/8] mkenvimage: More error handling

[David Wagner]

Verbosly fail if the target environment size or the padding byte are badly
formated.

Verbosly fail if something bad happens when reading from standard input.

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |   32 ++++++++++++++++++++++++++++++--
 1 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index bc18736..eb9a8f2 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -93,7 +93,17 @@ int main(int argc, char **argv)
 		switch (option) {
 		case 's':
 			datasize = strtol(optarg, NULL, 0);
-			break;
+			if (!errno)
+				break;
+
+			if (errno == ERANGE)
+				fprintf(stderr, "Bad integer format: %s\n",
+						optarg);
+			else
+				fprintf(stderr, "Error while parsing %s: %s\n",
+						optarg, strerror(errno));
+
+			return EXIT_FAILURE;
 		case 'o':
 			bin_filename = strdup(optarg);
 			if (!bin_filename) {
@@ -109,7 +119,17 @@ int main(int argc, char **argv)
 			break;
 		case 'p':
 			padbyte = strtol(optarg, NULL, 0);
-			break;
+			if (!errno)
+				break;
+
+			if (errno == ERANGE)
+				fprintf(stderr, "Bad integer format: %s\n",
+						optarg);
+			else
+				fprintf(stderr, "Error while parsing %s: %s\n",
+						optarg, strerror(errno));
+
+			return EXIT_FAILURE;
 		case 'h':
 			usage(prg);
 			return EXIT_SUCCESS;
@@ -166,7 +186,15 @@ int main(int argc, char **argv)
 
 		do {
 			filebuf = realloc(filebuf, readlen);
+			if (!filebuf) {
+				fprintf(stderr, "Can't realloc memory for the input file buffer\n");
+				return EXIT_FAILURE;
+			}
 			readbytes = read(txt_fd, filebuf + filesize, readlen);
+			if (errno) {
+				fprintf(stderr, "Error while reading stdin: %s\n", strerror(errno));
+				return EXIT_FAILURE;
+			}
 			filesize += readbytes;
 		} while (readbytes == readlen);
 
-- 
1.7.5.4

[U-Boot] [PATCHv3 5/8] mkenvimage: Read/Write from/to stdin/out by default or if the filename is "-"

[David Wagner]

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index eb9a8f2..6db2b21 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -173,15 +173,9 @@ int main(int argc, char **argv)
 	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) {
+	if (optind >= argc || strcmp(argv[optind], "-") == 0) {
 		int readbytes = 0;
-		int readlen = sizeof(*envptr) * 2048;
+		int readlen = sizeof(*envptr) * 4096;
 		txt_fd = STDIN_FILENO;
 
 		do {
@@ -199,6 +193,7 @@ int main(int argc, char **argv)
 		} while (readbytes == readlen);
 
 	} else {
+		txt_filename = argv[optind];
 		txt_fd = open(txt_filename, O_RDONLY);
 		if (txt_fd == -1) {
 			fprintf(stderr, "Can't open \"%s\": %s\n",
@@ -288,11 +283,16 @@ int main(int argc, char **argv)
 
 	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 (!bin_filename || strcmp(bin_filename, "-") == 0) {
+		bin_fd = STDOUT_FILENO;
+	} else {
+		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) !=
-- 
1.7.5.4

[U-Boot] [PATCHv2 6/8] mkenvimage: Use mmap() when reading from a regular file

[David Wagner]

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index 6db2b21..58f1d0b 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -38,6 +38,7 @@
 #include "compiler.h"
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/mman.h>
 
 #include "u-boot/crc.h"
 #include <version.h>
@@ -209,14 +210,16 @@ int main(int argc, char **argv)
 		}
 
 		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");
+
+		filebuf = mmap(NULL, sizeof(*envptr) * filesize, PROT_READ,
+			       MAP_PRIVATE, txt_fd, 0);
+		ret = close(txt_fd);
+		if (filebuf == MAP_FAILED) {
+			fprintf(stderr, "mmap (%ld bytes) failed: %s\n",
+					sizeof(*envptr) * filesize,
+					strerror(errno));
 			return EXIT_FAILURE;
 		}
-		ret = close(txt_fd);
 	}
 	/* The +1 is for the additionnal ending \0. See below. */
 	if (filesize + 1 > envsize) {
-- 
1.7.5.4

[U-Boot] [PATCHv2 7/8] mkenvimage: Don't try to detect comments in the input file

[David Wagner]

Remove this feature since it seems impossible to reliably detect them.

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index 58f1d0b..a3d4e27 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -249,14 +249,6 @@ int main(int argc, char **argv)
 				/* 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];
 		}
-- 
1.7.5.4

[U-Boot] [PATCHv3 8/8] mkenvimage: Really set the redundant byte when applicable

[David Wagner]

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index a3d4e27..c1e6cad 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -276,7 +276,9 @@ int main(int argc, char **argv)
 	crc = crc32(0, envptr, envsize);
 	targetendian_crc = bigendian ? cpu_to_be32(crc) : cpu_to_le32(crc);
 
-	memcpy(dataptr, &targetendian_crc, sizeof(uint32_t));
+	memcpy(dataptr, &targetendian_crc, sizeof(targetendian_crc));
+	if (redundant)
+		dataptr[sizeof(targetendian_crc)] = 1;
 
 	if (!bin_filename || strcmp(bin_filename, "-") == 0) {
 		bin_fd = STDOUT_FILENO;
-- 
1.7.5.4

[U-Boot] [PATCHv2 1/8] Strip mkenvimage

[Mike Frysinger]

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/20120108/5c5b67da/attachment.pgp>

[U-Boot] [PATCHv3 3/8] mkenvimage: Correct the includes and add a missing one

[Mike Frysinger]

On Thursday 05 January 2012 13:44:54 David Wagner wrote:
> --- a/tools/mkenvimage.c
> +++ b/tools/mkenvimage.c
>
> +#include <stdlib.h>
>
> -#include <compiler.h>
> +#include "compiler.h"

these are fine

> -#include <u-boot/crc.h>
> +#include "u-boot/crc.h"

i don't think this is necessary
-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/20120108/64167dc6/attachment.pgp>

[U-Boot] [PATCHv2 4/8] mkenvimage: More error handling

[Mike Frysinger]

On Thursday 05 January 2012 13:44:55 David Wagner wrote:
> --- a/tools/mkenvimage.c
> +++ b/tools/mkenvimage.c
>
>  			datasize = strtol(optarg, NULL, 0);
> -			break;
> +			if (!errno)
> +				break;
> +
> +			if (errno == ERANGE)
> +				fprintf(stderr, "Bad integer format: %s\n",
> +						optarg);
> +			else
> +				fprintf(stderr, "Error while parsing %s: %s\n",
> +						optarg, strerror(errno));
> +
> +			return EXIT_FAILURE;

seems like this should be a local xstrol() helper
-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/20120108/79159b3b/attachment.pgp>

[U-Boot] [PATCHv3 5/8] mkenvimage: Read/Write from/to stdin/out by default or if the filename is "-"

[Mike Frysinger]

On Thursday 05 January 2012 13:44:56 David Wagner wrote:
> +		bin_fd = creat(bin_filename, S_IRUSR | S_IWUSR | S_IRGRP |
> +					     S_IWGRP);

this should prob be open()
-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/20120108/27e60eca/attachment.pgp>

[U-Boot] [PATCHv2 6/8] mkenvimage: Use mmap() when reading from a regular file

[Mike Frysinger]

On Thursday 05 January 2012 13:44:57 David Wagner wrote:
> --- a/tools/mkenvimage.c
> +++ b/tools/mkenvimage.c
> 
>  		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");
> +
> +		filebuf = mmap(NULL, sizeof(*envptr) * filesize, PROT_READ,
> +			       MAP_PRIVATE, txt_fd, 0);
> +		ret = close(txt_fd);
> +		if (filebuf == MAP_FAILED) {
> +			fprintf(stderr, "mmap (%ld bytes) failed: %s\n",
> +					sizeof(*envptr) * filesize,
> +					strerror(errno));
>  			return EXIT_FAILURE;
>  		}
> -		ret = close(txt_fd);

seems like the mmap() failure shouldn't be fatal.  just have it fallback to 
the normal read()/write() logic.
-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/20120108/88943d68/attachment.pgp>

[U-Boot] [PATCHv3 5/8] mkenvimage: Read/Write from/to stdin/out by default or if the filename is "-"

[David Wagner]

Le 08/01/2012 07:50, Mike Frysinger a ?crit :
> On Thursday 05 January 2012 13:44:56 David Wagner wrote:
>> +		bin_fd = creat(bin_filename, S_IRUSR | S_IWUSR | S_IRGRP |
>> +					     S_IWGRP);
>
> this should prob be open()
> -mike

What is wrong with creat() ?
from 'man 2 creat': creat() is equivalent to open() with flags equal to 
O_CREAT|O_WRONLY|O_TRUNC.
We want to create the file if it doesn't exist and truncate it if it does.

The only issue I can think of is that no additional flag can be passed 
to creat(); fcntl can alter some of them.  But will we need any ?

Regards,
David.

[U-Boot] [PATCHv4 3/8] mkenvimage: Correct an include and add a missing one

[David Wagner]

compiler.h needs to be included from U-Boot's headers.
Also, group U-Boot-specific includes together

stdlib.h was missing.

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index c95f7f5..810a89e 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -31,13 +31,14 @@
 #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 "compiler.h"
 #include <u-boot/crc.h>
 #include <version.h>
 
-- 
1.7.5.4

[U-Boot] [PATCHv3 6/8] mkenvimage: Use mmap() when reading from a regular file

[David Wagner]

Fall back to read() if it fails.

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

I let an error message be printed when mmap fails, as the user may want to know
why.

 tools/mkenvimage.c |   25 +++++++++++++++++++------
 1 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index 20e53b8..ca32df7 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -37,6 +37,7 @@
 #include <unistd.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/mman.h>
 
 #include "compiler.h"
 #include <u-boot/crc.h>
@@ -210,12 +211,24 @@ int main(int argc, char **argv)
 		}
 
 		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;
+
+		filebuf = mmap(NULL, sizeof(*envptr) * filesize, PROT_READ,
+			       MAP_PRIVATE, txt_fd, 0);
+		if (filebuf == MAP_FAILED) {
+			fprintf(stderr, "mmap (%ld bytes) failed: %s\n",
+					sizeof(*envptr) * filesize,
+					strerror(errno));
+			fprintf(stderr, "Falling back to read()\n");
+
+			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 (%ld bytes): %s\n",
+					sizeof(*envptr) * filesize,
+					strerror(errno));
+
+				return EXIT_FAILURE;
+			}
 		}
 		ret = close(txt_fd);
 	}
-- 
1.7.5.4

[U-Boot] [PATCHv3 4/8] mkenvimage: More error handling

[David Wagner]

Verbosly fail if the target environment size or the padding byte are badly
formated.

Verbosly fail if something bad happens when reading from standard input.

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |   30 ++++++++++++++++++++++++++++--
 1 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index 810a89e..9f2490e 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -63,6 +63,23 @@ static void usage(const char *exec_name)
 	       exec_name);
 }
 
+long int xstrtol(char *s)
+{
+	long int tmp;
+
+	tmp = strtol(s, NULL, 0);
+	if (!errno)
+		return tmp;
+
+	if (errno == ERANGE)
+		fprintf(stderr, "Bad integer format: %s\n",  s);
+	else
+		fprintf(stderr, "Error while parsing %s: %s\n", s,
+				strerror(errno));
+
+	exit(EXIT_FAILURE);
+}
+
 int main(int argc, char **argv)
 {
 	uint32_t crc, targetendian_crc;
@@ -92,7 +109,7 @@ int main(int argc, char **argv)
 	while ((option = getopt(argc, argv, ":s:o:rbp:hV")) != -1) {
 		switch (option) {
 		case 's':
-			datasize = strtol(optarg, NULL, 0);
+			datasize = xstrtol(optarg);
 			break;
 		case 'o':
 			bin_filename = strdup(optarg);
@@ -108,7 +125,7 @@ int main(int argc, char **argv)
 			bigendian = 1;
 			break;
 		case 'p':
-			padbyte = strtol(optarg, NULL, 0);
+			padbyte = xstrtol(optarg);
 			break;
 		case 'h':
 			usage(prg);
@@ -166,7 +183,16 @@ int main(int argc, char **argv)
 
 		do {
 			filebuf = realloc(filebuf, readlen);
+			if (!filebuf) {
+				fprintf(stderr, "Can't realloc memory for the input file buffer\n");
+				return EXIT_FAILURE;
+			}
 			readbytes = read(txt_fd, filebuf + filesize, readlen);
+			if (errno) {
+				fprintf(stderr, "Error while reading stdin: %s\n",
+						strerror(errno));
+				return EXIT_FAILURE;
+			}
 			filesize += readbytes;
 		} while (readbytes == readlen);
 
-- 
1.7.5.4

[U-Boot] [PATCHv4 3/8] mkenvimage: Correct an include and add a missing one

[Mike Frysinger]

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/20120108/38b03b5a/attachment.pgp>

[U-Boot] [PATCHv3 4/8] mkenvimage: More error handling

[Mike Frysinger]

On Sunday 08 January 2012 09:25:21 David Wagner wrote:
> --- a/tools/mkenvimage.c
> +++ b/tools/mkenvimage.c
> 
> +long int xstrtol(char *s)

const

> +{
> +	long int tmp;
> +
> +	tmp = strtol(s, NULL, 0);
> +	if (!errno)
> +		return tmp;

you should manually clear errno before calling strtol.  it'll set the value, 
but won't clear 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/20120108/9be14991/attachment.pgp>

[U-Boot] [PATCHv3 5/8] mkenvimage: Read/Write from/to stdin/out by default or if the filename is "-"

[Mike Frysinger]

On Sunday 08 January 2012 07:02:40 David Wagner wrote:
> Le 08/01/2012 07:50, Mike Frysinger a ?crit :
> > On Thursday 05 January 2012 13:44:56 David Wagner wrote:
> >> +		bin_fd = creat(bin_filename, S_IRUSR | S_IWUSR | S_IRGRP |
> >> +					     S_IWGRP);
> > 
> > this should prob be open()
> 
> What is wrong with creat() ?

nothings wrong with it per-say ... just unusual ;).  if you really want to 
keep it, then it's fine.
-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/20120108/bfd304e7/attachment.pgp>

[U-Boot] [PATCHv4 4/8] mkenvimage: More error handling

[David Wagner]

Verbosly fail if the target environment size or the padding byte are badly
formated.

Verbosly fail if something bad happens when reading from standard input.

Signed-off-by: David Wagner <david.wagner@free-electrons.com>
---
 tools/mkenvimage.c |   31 +++++++++++++++++++++++++++++--
 1 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/tools/mkenvimage.c b/tools/mkenvimage.c
index 810a89e..cc1deec 100644
--- a/tools/mkenvimage.c
+++ b/tools/mkenvimage.c
@@ -63,6 +63,24 @@ static void usage(const char *exec_name)
 	       exec_name);
 }
 
+long int xstrtol(const char *s)
+{
+	long int tmp;
+
+	errno = 0;
+	tmp = strtol(s, NULL, 0);
+	if (!errno)
+		return tmp;
+
+	if (errno == ERANGE)
+		fprintf(stderr, "Bad integer format: %s\n",  s);
+	else
+		fprintf(stderr, "Error while parsing %s: %s\n", s,
+				strerror(errno));
+
+	exit(EXIT_FAILURE);
+}
+
 int main(int argc, char **argv)
 {
 	uint32_t crc, targetendian_crc;
@@ -92,7 +110,7 @@ int main(int argc, char **argv)
 	while ((option = getopt(argc, argv, ":s:o:rbp:hV")) != -1) {
 		switch (option) {
 		case 's':
-			datasize = strtol(optarg, NULL, 0);
+			datasize = xstrtol(optarg);
 			break;
 		case 'o':
 			bin_filename = strdup(optarg);
@@ -108,7 +126,7 @@ int main(int argc, char **argv)
 			bigendian = 1;
 			break;
 		case 'p':
-			padbyte = strtol(optarg, NULL, 0);
+			padbyte = xstrtol(optarg);
 			break;
 		case 'h':
 			usage(prg);
@@ -166,7 +184,16 @@ int main(int argc, char **argv)
 
 		do {
 			filebuf = realloc(filebuf, readlen);
+			if (!filebuf) {
+				fprintf(stderr, "Can't realloc memory for the input file buffer\n");
+				return EXIT_FAILURE;
+			}
 			readbytes = read(txt_fd, filebuf + filesize, readlen);
+			if (errno) {
+				fprintf(stderr, "Error while reading stdin: %s\n",
+						strerror(errno));
+				return EXIT_FAILURE;
+			}
 			filesize += readbytes;
 		} while (readbytes == readlen);
 
-- 
1.7.5.4

[U-Boot] [PATCH 1/9] Strip mkenvimage

[Wolfgang Denk]

Dear David Wagner,

In message <1322080098-3151-2-git-send-email-david.wagner@free-electrons.com> you wrote:
> Signed-off-by: David Wagner <david.wagner@free-electrons.com>
> ---
>  tools/Makefile |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)

Applied, 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
Drun'? 'm not drun'! You woudn' dare call m' drun' if I was sober!
                                     - Terry Pratchett, _Men at Arms_

[U-Boot] [PATCH 0/9] mkenvimage: v10/v11 diff split into several patches

[Wolfgang Denk]

Dear David Wagner,

In message <1322080098-3151-1-git-send-email-david.wagner@free-electrons.com> you wrote:
> 
> First, thanks for having mainlined mkenvimage :)
> 
> These patches mainly address Wolfgang's remarks.  There are a couple others.
> 
> I first intended to submit a v11 patch but since the v10 was merged into
> u-boot's master branch, I splitted the changes into 9 patches against it.

Sorry, but this patch series has become a mess; we have the original
1-9 series, then a v2 1-10, then v3 1-8, and even a v4 1-8, with htese
later series all being sparse ones.  Sonce the number of patches
changed several times, it's totally unclear to me which would be the
correct series to apply. 

Please rebase and post a cleaned up complete series.  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
The biggest difference between time and space is that you can't reuse
time.                                                 - Merrick Furst

[U-Boot] [PATCH 0/9] mkenvimage: v10/v11 diff split into several patches

[David Wagner]

Le 13/01/2012 21:07, Wolfgang Denk a ?crit :
> Sorry, but this patch series has become a mess; we have the original
> 1-9 series, then a v2 1-10, then v3 1-8, and even a v4 1-8, with htese
> later series all being sparse ones.  Sonce the number of patches
> changed several times, it's totally unclear to me which would be the
> correct series to apply.
>
> Please rebase and post a cleaned up complete series.  Thanks.
>
> Best regards,
>
> Wolfgang Denk
>

Hi,

All right, I'll send a "v5" for all the patches.

Best regards,
David.