Skip to contentSkip to conversation

mkenvimage: v10/v11 diff split into several patches

PATCH Superseded David Wagner 5 participants 47 messages Nov 23, 2011 – Jan 13, 2012

lore
Share & tools
lore
#0 lore raw

cover mkenvimage: v10/v11 diff split into several patches

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(-)
Signature · 1 line
1.7.7.3
#1 lore raw

PATCH 1/9 Strip mkenvimage

---
 tools/Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
tools/Makefile +1 -0
index a5f989a..64bcc4d 100644
@@ -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 \
Signature · 1 line
1.7.7.3
#2 lore raw

PATCH 1/9 Strip mkenvimage

Dear David Wagner,

Quoted 4 lines · In message <[email protected]>…
---
 tools/Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

Signature · 5 lines
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_
#3 lore raw

PATCH 2/9 mkenvimage: correct and clarify comments and error messages

In reply to David Wagner

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

---
 tools/mkenvimage.c |   39 ++++++++++++++-------------------------
 1 files changed, 14 insertions(+), 25 deletions(-)
tools/mkenvimage.c +14 -25
index 9c32f4a..da54658 100644
@@ -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';
Signature · 1 line
1.7.7.3
#4 lore raw

PATCH v2 2/10 mkenvimage: correct and clarify comments and error messages

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

---

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(-)
tools/mkenvimage.c +17 -18
index 753d9e6..5f7d6ea 100644
@@ -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';
Signature · 1 line
1.7.5.4
#5 lore raw

PATCH v2 2/10 mkenvimage: correct and clarify comments and error messages

Le Wed, 21 Dec 2011 01:58:25 +0100,
David Wagner <[email protected]> a ?crit :

tools/mkenvimage.c int main(int argc, char **argv) · line 203

Quoted 13 lines

+<<<<<<< HEAD

tools/mkenvimage.c +5 -0
 	/*
 	 * 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

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#6 lore raw

PATCH v2 2/10 mkenvimage: correct and clarify comments and error messages

Le 21/12/2011 08:41, Thomas Petazzoni a ?crit :

Quoted 6 lines

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.

#7 lore raw

PATCH 3/9 mkenvimage: Correct the includes and add a missing one

In reply to David Wagner

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

stdlib.h was missing.

---
 tools/mkenvimage.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
tools/mkenvimage.c +3 -2
index da54658..6af0d56 100644
@@ -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)
 
Signature · 1 line
1.7.7.3
#8 lore raw

PATCH v2 3/10 mkenvimage: Correct the includes and add a missing one

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

stdlib.h was missing.

---

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(-)
tools/mkenvimage.c +3 -12
index 5f7d6ea..b8b5c3a 100644
@@ -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;
 	}
 
Signature · 1 line
1.7.5.4
#9 lore raw

PATCH v2 3/10 mkenvimage: Correct the includes and add a missing one

Dear David Wagner,

Quoted 9 lines · In message <1324429120-10141-1-git-send-email-david.wagner@free-electrons.com>…

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

stdlib.h was missing.

---

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

Signature · 5 lines
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
#10 lore raw

PATCH v2 1/8 Strip mkenvimage

---
 tools/Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
tools/Makefile +1 -0
index a5f989a..64bcc4d 100644
@@ -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 \
Signature · 1 line
1.7.5.4
#11 lore raw

PATCH v3 2/8 mkenvimage: correct and clarify comments and error messages

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

---
 tools/mkenvimage.c |   41 +++++++++++++++--------------------------
 1 files changed, 15 insertions(+), 26 deletions(-)
tools/mkenvimage.c +15 -26
index c5ed373..7d33143 100644
@@ -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';
Signature · 1 line
1.7.5.4
#12 lore raw

PATCH v3 3/8 mkenvimage: Correct the includes and add a missing one

In reply to David Wagner

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

stdlib.h was missing.

---
 tools/mkenvimage.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
tools/mkenvimage.c +3 -2
index 7d33143..bc18736 100644
@@ -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)
Signature · 1 line
1.7.5.4
#13 lore raw

PATCH v3 3/8 mkenvimage: Correct the includes and add a missing one

tools/mkenvimage.c line 38

Quoted 7 lines · On Thursday 05 January 2012 13:44:54 David Wagner wrote:
tools/mkenvimage.c +2 -1
 
+#include <stdlib.h>
 
-#include <compiler.h>
+#include "compiler.h"

these are fine

tools/mkenvimage.c line 42

Quoted 2 lines
tools/mkenvimage.c +1 -1
-#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>

#14 lore raw

PATCH v4 3/8 mkenvimage: Correct an include and add a missing one

In reply to David Wagner

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

stdlib.h was missing.

---
 tools/mkenvimage.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
tools/mkenvimage.c +2 -1
index c95f7f5..810a89e 100644
@@ -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>
 
Signature · 1 line
1.7.5.4
#16 lore raw

PATCH v2 4/8 mkenvimage: More error handling

In reply to 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.

---
 tools/mkenvimage.c |   32 ++++++++++++++++++++++++++++++--
 1 files changed, 30 insertions(+), 2 deletions(-)
tools/mkenvimage.c +30 -2
index bc18736..eb9a8f2 100644
@@ -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);
 
Signature · 1 line
1.7.5.4
#17 lore raw

PATCH v2 4/8 mkenvimage: More error handling

Quoted 6 lines · On Thursday 05 January 2012 13:44:55 David Wagner wrote:
tools/mkenvimage.c +1 -1
 
 			datasize = strtol(optarg, NULL, 0);
-			break;
+			if (!errno)

tools/mkenvimage.c int main(int argc, char **argv) · line 106

Quoted 10 lines
tools/mkenvimage.c +10 -0
+				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>

#18 lore raw

PATCH v3 4/8 mkenvimage: More error handling

In reply to 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.

---
 tools/mkenvimage.c |   30 ++++++++++++++++++++++++++++--
 1 files changed, 28 insertions(+), 2 deletions(-)
tools/mkenvimage.c +28 -2
index 810a89e..9f2490e 100644
@@ -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);
 
Signature · 1 line
1.7.5.4
#19 lore raw

PATCH v3 4/8 mkenvimage: More error handling

tools/mkenvimage.c static void usage(const char *exec_name) · line 66

Quoted 4 lines · On Sunday 08 January 2012 09:25:21 David Wagner wrote:
tools/mkenvimage.c +1 -0
 
+long int xstrtol(char *s)

const

tools/mkenvimage.c static void usage(const char *exec_name) · line 72

Quoted 6 lines
tools/mkenvimage.c +6 -0
+{
+	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>

#20 lore raw

PATCH v4 4/8 mkenvimage: More error handling

In reply to 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.

---
 tools/mkenvimage.c |   31 +++++++++++++++++++++++++++++--
 1 files changed, 29 insertions(+), 2 deletions(-)
tools/mkenvimage.c +29 -2
index 810a89e..cc1deec 100644
@@ -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);
 
Signature · 1 line
1.7.5.4
#21 lore raw

PATCH v3 5/8 mkenvimage: Read/Write from/to stdin/out by default or if the filename is "-"

In reply to David Wagner
---
 tools/mkenvimage.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)
tools/mkenvimage.c +13 -13
index eb9a8f2..6db2b21 100644
@@ -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) !=
Signature · 1 line
1.7.5.4
#22 lore raw

PATCH v3 5/8 mkenvimage: Read/Write from/to stdin/out by default or if the filename is "-"

tools/mkenvimage.c int main(int argc, char **argv) · line 290

Quoted 2 lines · On Thursday 05 January 2012 13:44:56 David Wagner wrote:
tools/mkenvimage.c +2 -0
+		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>

#23 lore raw

PATCH v3 5/8 mkenvimage: Read/Write from/to stdin/out by default or if the filename is "-"

Le 08/01/2012 07:50, Mike Frysinger a ?crit :

Quoted 6 lines · On Thursday 05 January 2012 13:44:56 David Wagner wrote:

tools/mkenvimage.c int main(int argc, char **argv) · line 290

Quoted 2 lines
tools/mkenvimage.c +2 -0
+		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.

#24 lore raw

PATCH v3 5/8 mkenvimage: Read/Write from/to stdin/out by default or if the filename is "-"

Quoted 8 lines · On Sunday 08 January 2012 07:02:40 David Wagner wrote:

Le 08/01/2012 07:50, Mike Frysinger a ?crit :

Quoted 5 lines · On Thursday 05 January 2012 13:44:56 David Wagner wrote:

tools/mkenvimage.c int main(int argc, char **argv) · line 290

Quoted 2 lines
tools/mkenvimage.c +2 -0
+		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>

#25 lore raw

PATCH v2 6/8 mkenvimage: Use mmap() when reading from a regular file

In reply to David Wagner
---
 tools/mkenvimage.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)
tools/mkenvimage.c +9 -6
index 6db2b21..58f1d0b 100644
@@ -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) {
Signature · 1 line
1.7.5.4
#26 lore raw

PATCH v2 6/8 mkenvimage: Use mmap() when reading from a regular file

Quoted 10 lines · On Thursday 05 January 2012 13:44:57 David Wagner wrote:
tools/mkenvimage.c +1 -5
 
 		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");
+

tools/mkenvimage.c int main(int argc, char **argv) · line 219

Quoted 10 lines
tools/mkenvimage.c +7 -1
+		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>

#27 lore raw

PATCH v3 6/8 mkenvimage: Use mmap() when reading from a regular file

In reply to David Wagner

Fall back to read() if it fails.

---

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(-)
tools/mkenvimage.c +19 -6
index 20e53b8..ca32df7 100644
@@ -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);
 	}
Signature · 1 line
1.7.5.4
#28 lore raw

PATCH v2 7/8 mkenvimage: Don't try to detect comments in the input file

In reply to David Wagner

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

---
 tools/mkenvimage.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)
tools/mkenvimage.c +0 -8
index 58f1d0b..a3d4e27 100644
@@ -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];
 		}
Signature · 1 line
1.7.5.4
#29 lore raw

PATCH v3 8/8 mkenvimage: Really set the redundant byte when applicable

In reply to David Wagner
---
 tools/mkenvimage.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
tools/mkenvimage.c +3 -1
index a3d4e27..c1e6cad 100644
@@ -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;
Signature · 1 line
1.7.5.4
#31 lore raw

PATCH 4/9 mkenvimage: More error handling

In reply to 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.

---
 tools/mkenvimage.c |   32 ++++++++++++++++++++++++++++++--
 1 files changed, 30 insertions(+), 2 deletions(-)
tools/mkenvimage.c +30 -2
index 6af0d56..b6e7f14 100644
@@ -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);
 
Signature · 1 line
1.7.7.3
#32 lore raw

PATCH 5/9 mkenvimage: Read from stdin if the filename is "-"

In reply to David Wagner
---
 tools/mkenvimage.c |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)
tools/mkenvimage.c +3 -8
index b6e7f14..86a4e05 100644
@@ -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",
Signature · 1 line
1.7.7.3
#33 lore raw

PATCH v2 5/8 mkenvimage: Read/Write from/to stdin/out by default or if the filename is "-"

---

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

 tools/mkenvimage.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)
tools/mkenvimage.c +13 -13
index b6e7f14..ab67c1a 100644
@@ -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) !=
Signature · 1 line
1.7.7.3
#34 lore raw

PATCH 6/9 mkenvimage: Use mmap() when reading from a regular file

In reply to David Wagner
---
 tools/mkenvimage.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)
tools/mkenvimage.c +9 -6
index 86a4e05..74b296e 100644
@@ -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) {
Signature · 1 line
1.7.7.3
#35 lore raw

PATCH 7/9 mkenvimage: Don't try to detect comments in the input file

In reply to David Wagner

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

---
 tools/mkenvimage.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)
tools/mkenvimage.c +0 -8
index 74b296e..8d84c85 100644
@@ -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];
 		}
Signature · 1 line
1.7.7.3
#36 lore raw

PATCH 8/9 mkenvimage: Really set the redundant byte when applicable

In reply to David Wagner
---
 tools/mkenvimage.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
tools/mkenvimage.c +3 -1
index 8d84c85..fa6eb6e 100644
@@ -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) {
Signature · 1 line
1.7.7.3
#37 lore raw

PATCH 8/9 mkenvimage: Really set the redundant byte when applicable

tools/mkenvimage.c int main(int argc, char **argv) · line 262

Quoted 5 lines · On Wednesday 23 November 2011 15:28:17 David Wagner wrote:
tools/mkenvimage.c +2 -0
 
+	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>

#38 lore raw

PATCH v2 8/8 mkenvimage: Really set the redundant byte when applicable

In reply to David Wagner
---
 tools/mkenvimage.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
tools/mkenvimage.c +3 -1
index 221e9de..4e69056 100644
@@ -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;
Signature · 1 line
1.7.7.3
#39 lore raw

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

In reply to David Wagner
---
 tools/mkenvimage.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)
tools/mkenvimage.c +9 -5
index fa6eb6e..8ba63d2 100644
@@ -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) !=
Signature · 1 line
1.7.7.3
#40 lore raw

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

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>

#41 lore raw

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

Quoted 3 lines · Mike Frysinger

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.

#43 lore raw

cover mkenvimage: v10/v11 diff split into several patches

Le 27/11/2011 20:59, Mike Frysinger a ?crit :

Quoted 2 lines

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.

#44 lore raw

cover mkenvimage: v10/v11 diff split into several patches

Quoted 9 lines · David Wagner

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

Signature · 5 lines
=====================================================================
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
=====================================================================
#45 lore raw

cover mkenvimage: v10/v11 diff split into several patches

In reply to David Wagner

Dear David Wagner,

Quoted 7 lines · In message <[email protected]>…

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

Signature · 5 lines
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
#46 lore raw

cover mkenvimage: v10/v11 diff split into several patches

Le 13/01/2012 21:07, Wolfgang Denk a ?crit :

Quoted 6 lines

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.

Quoted 6 lines

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.