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

login
register
mail settings
Submitter David Wagner
Date Dec. 21, 2011, 12:58 a.m.
Message ID <1324429105-10104-1-git-send-email-david.wagner@free-electrons.com>
Download mbox | patch
Permalink /patch/132543/
State Not Applicable
Headers show

Comments

David Wagner - Dec. 21, 2011, 12:58 a.m.
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(-)
Thomas Petazzoni - Dec. 21, 2011, 7:41 a.m.
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
David Wagner - Dec. 21, 2011, 10:10 p.m.
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.

Patch

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';