[PATCH 1/8] nanddump/nandwrite: use "simple_" str functions

[PATCH 1/8] nanddump/nandwrite: use "simple_" str functions

[Brian Norris]

Per Mike Frysinger's suggestion, we check for strtoll() and strtoull()
errors by using the "common.h" helper functions simple_strtoll() and
simple_strtoull().

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nanddump.c  |    4 ++--
 nandwrite.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index fe29596..bf95e81 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -136,7 +136,7 @@ static void process_options(int argc, char * const argv[])
 				omitbad = true;
 				break;
 			case 's':
-				start_addr = strtoull(optarg, NULL, 0);
+				start_addr = simple_strtoull(optarg, &error);
 				break;
 			case 'f':
 				if (!(dumpfile = strdup(optarg))) {
@@ -145,7 +145,7 @@ static void process_options(int argc, char * const argv[])
 				}
 				break;
 			case 'l':
-				length = strtoull(optarg, NULL, 0);
+				length = simple_strtoull(optarg, &error);
 				break;
 			case 'o':
 				omitoob = true;
diff --git a/nandwrite.c b/nandwrite.c
index bbe69b0..5373a89 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -201,7 +201,7 @@ static void process_options(int argc, char * const argv[])
 				writeoob = true;
 				break;
 			case 's':
-				mtdoffset = strtoll(optarg, NULL, 0);
+				mtdoffset = simple_strtoll(optarg, &error);
 				break;
 			case 'b':
 				blockalign = atoi(optarg);
-- 
1.7.0.4

[PATCH 2/8] mtd-utils: common.h: Add MAX() macro, fix MIN()

[Brian Norris]

Add MAX() macro to common.h, to be used in future patches.

Also a style change in comma location on MIN().

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 include/common.h |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/include/common.h b/include/common.h
index 25abc0c..eb8d5fc 100644
--- a/include/common.h
+++ b/include/common.h
@@ -34,7 +34,10 @@ extern "C" {
 #endif
 
 #ifndef MIN	/* some C lib headers define this for us */
-#define MIN(a ,b) ((a) < (b) ? (a) : (b))
+#define MIN(a, b) ((a) < (b) ? (a) : (b))
+#endif
+#ifndef MAX
+#define MAX(a, b) ((a) > (b) ? (a) : (b))
 #endif
 #define min(a, b) MIN(a, b) /* glue for linux kernel source */
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
-- 
1.7.0.4

[PATCH 3/8] nanddump: Refactor pretty print code into an sprintf()

[Brian Norris]

A do-while loop in pretty_dump_to_buffer() can be refactored into a
single sprintf() statement. MAX() and MIN() are used to ensure that:
(1) We have at least a single space between hex and ASCII output
(2) We don't overflow the line buffer

This patch was suggested by Mike Frysinger.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nanddump.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index bf95e81..14a8816 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -252,9 +252,10 @@ static void pretty_dump_to_buffer(const unsigned char *buf, size_t len,
 	if (!ascii)
 		goto nil;
 
-	do {
-		linebuf[lx++] = ' ';
-	} while (lx < (linebuflen - 1) && lx < (ascii_column - 1));
+	/* Spacing between hex and ASCII - ensure at least one space */
+	lx += sprintf(linebuf + lx, "%*s",
+			MAX((int)MIN(linebuflen, ascii_column) - 1 - lx, 1),
+			" ");
 
 	linebuf[lx++] = '|';
 	for (j = 0; (j < len) && (lx + 2) < linebuflen; j++)
-- 
1.7.0.4

[PATCH 4/8] nanddump: change "unsigned" to "signed"

[Brian Norris]

For consistency between nanddump and nandwrite and in order to provide
better means for checking for negative inputs, the "offset" and "length"
types in nanddump should be changed to signed integer types. This also
solves a signed/unsigned comparison warning.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nanddump.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index 14a8816..b0dd7dc 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -82,8 +82,8 @@ static bool			pretty_print = false;	// print nice
 static bool			noecc = false;		// don't error correct
 static bool			noskipbad = false;	// don't skip bad blocks
 static bool			omitoob = false;	// omit oob data
-static unsigned long long	start_addr;		// start address
-static unsigned long long	length;			// dump length
+static long long		start_addr;		// start address
+static long long		length;			// dump length
 static const char		*mtddev;		// mtd device name
 static const char		*dumpfile;		// dump file name
 static bool			omitbad = false;
@@ -136,7 +136,7 @@ static void process_options(int argc, char * const argv[])
 				omitbad = true;
 				break;
 			case 's':
-				start_addr = simple_strtoull(optarg, &error);
+				start_addr = simple_strtoll(optarg, &error);
 				break;
 			case 'f':
 				if (!(dumpfile = strdup(optarg))) {
@@ -145,7 +145,7 @@ static void process_options(int argc, char * const argv[])
 				}
 				break;
 			case 'l':
-				length = simple_strtoull(optarg, &error);
+				length = simple_strtoll(optarg, &error);
 				break;
 			case 'o':
 				omitoob = true;
@@ -273,8 +273,8 @@ nil:
  */
 int main(int argc, char * const argv[])
 {
-	unsigned long long ofs, end_addr = 0;
-	unsigned long long blockstart = 1;
+	long long ofs, end_addr = 0;
+	long long blockstart = 1;
 	int ret, i, fd, ofd = 0, bs, badblock = 0;
 	struct mtd_dev_info mtd;
 	char pretty_buf[PRETTY_BUF_LEN];
-- 
1.7.0.4

[PATCH 5/8] nanddump: check for negative inputs

[Brian Norris]

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nanddump.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index b0dd7dc..96b46d4 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -173,6 +173,13 @@ static void process_options(int argc, char * const argv[])
 		}
 	}
 
+	if (start_addr < 0)
+		errmsg_die("Can't specify a negative device offset '%lld'",
+				start_addr);
+
+	if (length < 0)
+		errmsg_die("Can't specify a negative length '%lld'", length);
+
 	if (quiet && pretty_print) {
 		fprintf(stderr, "The quiet and pretty print options are mutually-\n"
 				"exclusive. Choose one or the other.\n");
-- 
1.7.0.4

[PATCH v2 5/8] nanddump: check for negative inputs

[Brian Norris]

Includes error messages for negative device offsets and negative lengths,
telling the user what the offending option and value were.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nanddump.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index b0dd7dc..9f52878 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -173,6 +173,13 @@ static void process_options(int argc, char * const argv[])
 		}
 	}
 
+	if (start_addr < 0)
+		errmsg_die("Can't specify negative offset with option -s: %lld",
+				start_addr);
+
+	if (length < 0)
+		errmsg_die("Can't specify length with option -l: %lld", length);
+
 	if (quiet && pretty_print) {
 		fprintf(stderr, "The quiet and pretty print options are mutually-\n"
 				"exclusive. Choose one or the other.\n");
-- 
1.7.0.4

[PATCH v3 5/8] nanddump: check for negative inputs

[Brian Norris]

Includes error messages for negative device offsets and negative lengths,
telling the user what the offending option and value were.

Previous patch left out the "negative" in the error message.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nanddump.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index b0dd7dc..6a6f904 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -173,6 +173,13 @@ static void process_options(int argc, char * const argv[])
 		}
 	}
 
+	if (start_addr < 0)
+		errmsg_die("Can't specify negative offset with option -s: %lld",
+				start_addr);
+
+	if (length < 0)
+		errmsg_die("Can't specify negative length with option -l: %lld", length);
+
 	if (quiet && pretty_print) {
 		fprintf(stderr, "The quiet and pretty print options are mutually-\n"
 				"exclusive. Choose one or the other.\n");
-- 
1.7.0.4

[PATCH 6/8] nanddump: choose correct "printf" format-specifier

[Brian Norris]

The mtd-descriptor attributes contain signed data, not unsigned.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nanddump.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index 96b46d4..0e393cc 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -381,7 +381,7 @@ int main(int argc, char * const argv[])
 
 	/* Print informative message */
 	if (!quiet) {
-		fprintf(stderr, "Block size %u, page size %u, OOB size %u\n",
+		fprintf(stderr, "Block size %d, page size %d, OOB size %d\n",
 				mtd.eb_size, mtd.min_io_size, mtd.oob_size);
 		fprintf(stderr,
 				"Dumping data starting at 0x%08llx and ending at 0x%08llx...\n",
-- 
1.7.0.4

[PATCH 7/8] nandwrite: add check for negative blockalign

[Brian Norris]

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nandwrite.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/nandwrite.c b/nandwrite.c
index 5373a89..c409878 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -218,6 +218,10 @@ static void process_options(int argc, char * const argv[])
 		exit(EXIT_FAILURE);
 	}
 
+	if (blockalign < 0)
+		errmsg_die("Can't specify a negative blockalign '%d'",
+				blockalign);
+
 	argc -= optind;
 	argv += optind;
 
-- 
1.7.0.4

Re: [PATCH 7/8] nandwrite: add check for negative blockalign

[Mike Frysinger]

On Mon, Nov 29, 2010 at 03:02, Brian Norris wrote:
> +               errmsg_die("Can't specify a negative blockalign '%d'",
> +                               blockalign);

personally, i'd avoid quotes on non-string types.  maybe also mention
the command line option that is "blockalign" ?

otherwise, these changes all look fine to me
-mike

Re: [PATCH 7/8] nandwrite: add check for negative blockalign

[Brian Norris]

On 11/29/2010 12:05 AM, Mike Frysinger wrote:
> On Mon, Nov 29, 2010 at 03:02, Brian Norris wrote:
>> +               errmsg_die("Can't specify a negative blockalign '%d'",
>> +                               blockalign);
> 
> personally, i'd avoid quotes on non-string types.  maybe also mention
> the command line option that is "blockalign" ?

I'm sending some "v2" patches related to these err messages. I'm not
sure what the best, descriptive messages are, but I tried.

Brian

[PATCH v2 7/8] nandwrite: add check for negative blockalign

[Brian Norris]

Includes error messages for negative blockalign, telling the user what
the offending option and value were.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nandwrite.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/nandwrite.c b/nandwrite.c
index 5373a89..2e45136 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -218,6 +218,10 @@ static void process_options(int argc, char * const argv[])
 		exit(EXIT_FAILURE);
 	}
 
+	if (blockalign < 0)
+		errmsg_die("Can't specify negative blockalign with option -b:"
+				" %d", blockalign);
+
 	argc -= optind;
 	argv += optind;
 
-- 
1.7.0.4

[PATCH 8/8] nandwrite: use common.h "errmsg_die"

[Brian Norris]

errmsg_die() should be nearly the equivalent of the error message used
here. This saves a few lines.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nandwrite.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/nandwrite.c b/nandwrite.c
index c409878..c51efd7 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -212,11 +212,9 @@ static void process_options(int argc, char * const argv[])
 		}
 	}
 
-	if (mtdoffset < 0) {
-		fprintf(stderr, "Can't specify a negative device offset `%lld'\n",
+	if (mtdoffset < 0)
+		errmsg_die("Can't specify a negative device offset '%lld'",
 				mtdoffset);
-		exit(EXIT_FAILURE);
-	}
 
 	if (blockalign < 0)
 		errmsg_die("Can't specify a negative blockalign '%d'",
-- 
1.7.0.4

[PATCH v2 8/8] nandwrite: use common.h "errmsg_die"

[Brian Norris]

errmsg_die() should be nearly the equivalent of the error message used
here. This saves a few lines.

Also edited the error message to include the offending option and got
rid of the quotes.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nandwrite.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/nandwrite.c b/nandwrite.c
index 2e45136..7f459cb 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -212,11 +212,9 @@ static void process_options(int argc, char * const argv[])
 		}
 	}
 
-	if (mtdoffset < 0) {
-		fprintf(stderr, "Can't specify a negative device offset `%lld'\n",
-				mtdoffset);
-		exit(EXIT_FAILURE);
-	}
+	if (mtdoffset < 0)
+		errmsg_die("Can't specify negative device offset with option"
+				" -s: %lld", mtdoffset);
 
 	if (blockalign < 0)
 		errmsg_die("Can't specify negative blockalign with option -b:"
-- 
1.7.0.4

Re: [PATCH 1/8] nanddump/nandwrite: use "simple_" str functions

[Artem Bityutskiy]

On Mon, 2010-11-29 at 00:01 -0800, Brian Norris wrote:
> Per Mike Frysinger's suggestion, we check for strtoll() and strtoull()
> errors by using the "common.h" helper functions simple_strtoll() and
> simple_strtoull().
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Pushed the series, picked newest patches, thanks a lot!

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)