[PATCH] mtd-utils: formatting of odd-sized OOB in nanddump

[PATCH] mtd-utils: formatting of odd-sized OOB in nanddump

[Brian Norris]

Changed OOB output formatting in nanddump.
Most NAND devices have OOB regions with multiples of 16 bytes.
With newer devices, a 218-byte OOB does not split nicely into
lines of 16 bytes. This patch fixes the formatting, allowing a
last line of 10 bytes instead of 16.

Signed-off-by: Brian Norris <norris@broadcom.com>
---
 nanddump.c |   31 ++++++++++++++++++++++---------
 1 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index e44ab36..190a653 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -381,15 +381,28 @@ int main(int argc, char * const argv[])
 			}
 
 			for (i = 0; i < meminfo.oobsize; i += 16) {
-				sprintf(pretty_buf, "  OOB Data: %02x %02x %02x %02x %02x %02x "
-						"%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n",
-						oobbuf[i], oobbuf[i+1], oobbuf[i+2],
-						oobbuf[i+3], oobbuf[i+4], oobbuf[i+5],
-						oobbuf[i+6], oobbuf[i+7], oobbuf[i+8],
-						oobbuf[i+9], oobbuf[i+10], oobbuf[i+11],
-						oobbuf[i+12], oobbuf[i+13], oobbuf[i+14],
-						oobbuf[i+15]);
-				write(ofd, pretty_buf, 60);
+				/* Check if we have a full 16 bytes left in the OOB */
+				if (i+15 < meminfo.oobsize) {
+					/* Print 16 bytes */
+					sprintf(pretty_buf, "  OOB Data: %02x %02x %02x %02x %02x %02x "
+							"%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n",
+							oobbuf[i], oobbuf[i+1], oobbuf[i+2],
+							oobbuf[i+3], oobbuf[i+4], oobbuf[i+5],
+							oobbuf[i+6], oobbuf[i+7], oobbuf[i+8],
+							oobbuf[i+9], oobbuf[i+10], oobbuf[i+11],
+							oobbuf[i+12], oobbuf[i+13], oobbuf[i+14],
+							oobbuf[i+15]);
+					write(ofd, pretty_buf, 60);
+				} else {
+					/* Print 10 bytes */
+					sprintf(pretty_buf, "  OOB Data: %02x %02x %02x %02x %02x %02x "
+							"%02x %02x %02x %02x\n",
+							oobbuf[i], oobbuf[i+1], oobbuf[i+2],
+							oobbuf[i+3], oobbuf[i+4], oobbuf[i+5],
+							oobbuf[i+6], oobbuf[i+7], oobbuf[i+8],
+							oobbuf[i+9]);
+					write(ofd, pretty_buf, 42);
+				}
 			}
 		} else
 			write(ofd, oobbuf, meminfo.oobsize);
-- 
1.7.1

Re: [PATCH] mtd-utils: formatting of odd-sized OOB in nanddump

[Artem Bityutskiy]

On Wed, 2010-06-16 at 09:42 -0700, Brian Norris wrote:
> +				if (i+15 < meminfo.oobsize) {
> +					/* Print 16 bytes */
> +					sprintf(pretty_buf, "  OOB Data: %02x %02x %02x %02x %02x %02x "
> +							"%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n",
> +							oobbuf[i], oobbuf[i+1], oobbuf[i+2],
> +							oobbuf[i+3], oobbuf[i+4], oobbuf[i+5],
> +							oobbuf[i+6], oobbuf[i+7], oobbuf[i+8],
> +							oobbuf[i+9], oobbuf[i+10], oobbuf[i+11],
> +							oobbuf[i+12], oobbuf[i+13], oobbuf[i+14],
> +							oobbuf[i+15]);
> +					write(ofd, pretty_buf, 60);
> +				} else {
> +					/* Print 10 bytes */
> +					sprintf(pretty_buf, "  OOB Data: %02x %02x %02x %02x %02x %02x "
> +							"%02x %02x %02x %02x\n",
> +							oobbuf[i], oobbuf[i+1], oobbuf[i+2],
> +							oobbuf[i+3], oobbuf[i+4], oobbuf[i+5],
> +							oobbuf[i+6], oobbuf[i+7], oobbuf[i+8],
> +							oobbuf[i+9]);
> +					write(ofd, pretty_buf, 42);
> +				}

Why 10 bytes, why not 12 or 14?

I think it is better to just copy-paste-modify the kernel
print_hex_dump() and utilize it, instead of this ugly crocodile code ...

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

Re: [PATCH] mtd-utils: formatting of odd-sized OOB in nanddump

[Brian Norris]

> Why 10 bytes, why not 12 or 14?

As of now, the only OOB-size that's not a multiple of 16 supported in 
the other bits of ugly code is 218-bytes.

218 % 16 = 10

What's the point of checking for a "supported" page-size/OOB-size 
earlier if we're not gonna use that info? :)

> I think it is better to just copy-paste-modify the kernel
> print_hex_dump() and utilize it, instead of this ugly crocodile code ...

I'm looking at that, since it seems a better solution. Does anyone 
oppose including ASCII output in the "pretty" option? It's built in to 
the print_hex_dump() already, so it shouldn't be too difficult to adapt. 
Anyway, it seems that someone intended to print ASCII at some point 
(nanddump.c, line 77):

static bool             pretty_print = false;   // print nice in ascii

Re: [PATCH] mtd-utils: formatting of odd-sized OOB in nanddump

[Artem Bityutskiy]

Hi,

On Wed, 2010-07-07 at 11:21 -0700, Brian Norris wrote:
> > Why 10 bytes, why not 12 or 14?
> 
> As of now, the only OOB-size that's not a multiple of 16 supported in 
> the other bits of ugly code is 218-bytes.
> 
> 218 % 16 = 10
> 
> What's the point of checking for a "supported" page-size/OOB-size 
> earlier if we're not gonna use that info? :)

No point, as well as not much point introducing magic hard-coded
constant like 10 - the printing should be generic.
> 
> > I think it is better to just copy-paste-modify the kernel
> > print_hex_dump() and utilize it, instead of this ugly crocodile code ...
> 
> I'm looking at that, since it seems a better solution. Does anyone 
> oppose including ASCII output in the "pretty" option? It's built in to 
> the print_hex_dump() already, so it shouldn't be too difficult to adapt. 
> Anyway, it seems that someone intended to print ASCII at some point 
> (nanddump.c, line 77):
> 
> static bool             pretty_print = false;   // print nice in ascii

Well, it is better to make this configurable or not print ASCII, IMO,
because it potentially may break scripts which use nanddump.

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

[PATCH] mtd-utils update

[Brian Norris]

Hello,

This patch series includes a few different fixes. Patches 1, 2, 3,
and 6 are fixes to the nanddump utility, while 4 and 5 are style
and minor issue patches.

As per the recommendation, I adapted the linux kernel dump function for
nanddump (patch 2).

The patches are:

[PATCH 1/6] mtd-utils/nanddump.c: Increase max OOB size
[PATCH 2/6] mtd-utils/nanddump.c: Robust pretty hexdump
[PATCH 3/6] mtd-utils/nanddump.c: Add canonical (hex+ascii) flag
[PATCH 4/6] mtd-utils/mkfs.jffs2: fixed warnings
[PATCH 5/6] mtd-utils/nandtest.c: Fixed indentation
[PATCH 6/6] mtd-utils/nanddump.c: Add "forcebinary" flag

Feedback is welcome, especially on the feature additions of patches
2, 3 and 6.

Thanks,
Brian

[PATCH 1/6] mtd-utils/nanddump.c: Increase max OOB size

[Brian Norris]

Supported OOB and page sizes can now be changed more easily by a
macro constant. NAND_MAX_OOBSIZE needed increased to support 218
and 224 byte OOB.

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

diff --git a/nanddump.c b/nanddump.c
index e44ab36..735ae48 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -174,8 +174,10 @@ static void process_options (int argc, char * const argv[])
 /*
  * Buffers for reading data from flash
  */
-static unsigned char readbuf[4096];
-static unsigned char oobbuf[128];
+#define NAND_MAX_PAGESIZE 4096
+#define NAND_MAX_OOBSIZE 256
+static unsigned char readbuf[NAND_MAX_PAGESIZE];
+static unsigned char oobbuf[NAND_MAX_OOBSIZE];
 
 /*
  * Main program
-- 
1.7.0.4

Re: [PATCH 1/6] mtd-utils/nanddump.c: Increase max OOB size

[Artem Bityutskiy]

On Thu, 2010-07-08 at 13:50 -0700, Brian Norris wrote:
> Supported OOB and page sizes can now be changed more easily by a
> macro constant. NAND_MAX_OOBSIZE needed increased to support 218
> and 224 byte OOB.
> 
> Signed-off-by: Brian Norris <norris@broadcom.com>

Pushed to mtd-utils.git

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

[PATCH 2/6] mtd-utils/nanddump.c: Robust pretty hexdump

[Brian Norris]

Adapted code from the linux kernel hex_dump_to_buffer() (lib/hexdump.c)
to provide a more robust hexdump for the pretty option. Now, nanddump
can print out any size of OOB (or page for that matter...) without
having to worry about non-multiples of 16.

This also provides ability to dump ASCII format next to the hex output
once additional command-line flags are added.

Tested with Samsung K9GAG08U0D

Signed-off-by: Brian Norris <norris@broadcom.com>
---
 nanddump.c |  116 ++++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 82 insertions(+), 34 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index 735ae48..7281279 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -83,6 +83,7 @@ static const char	*mtddev;		// mtd device name
 static const char	*dumpfile;		// dump file name
 static bool		omitbad = false;
 static bool		quiet = false;		// suppress diagnostic output
+static bool		ascii = false;
 
 static void process_options (int argc, char * const argv[])
 {
@@ -171,6 +172,79 @@ static void process_options (int argc, char * const argv[])
 	mtddev = argv[optind];
 }
 
+#define PRETTY_ROW_SIZE 16
+#define PRETTY_BUF_LEN 80
+
+/**
+ * pretty_dump_to_buffer - formats a blob of data to "hex ASCII" in memory
+ * @buf: data blob to dump
+ * @len: number of bytes in the @buf
+ * @linebuf: where to put the converted data
+ * @linebuflen: total size of @linebuf, including space for terminating NULL
+ * @pagedump: true - dumping as page format; false - dumping as OOB format
+ * @prefix: address to print before line in a page dump, ignored if !pagedump
+ *
+ * pretty_dump_to_buffer() works on one "line" of output at a time, i.e.,
+ * PRETTY_ROW_SIZE bytes of input data converted to hex + ASCII output.
+ *
+ * Given a buffer of unsigned char data, pretty_dump_to_buffer() converts the
+ * input data to a hex/ASCII dump at the supplied memory location. A prefix
+ * is included based on whether we are dumping page or OOB data. The converted
+ * output is always NULL-terminated.
+ *
+ * e.g. with ascii = false:
+ *   pretty_dump_to_buffer(data, data_len, prettybuf, linelen,
+ *                         true, 256);
+ * produces:
+ *   0x00000100: 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f
+ * NOTE: This function was adapted from linux kernel, "lib/hexdump.c"
+ */
+static void pretty_dump_to_buffer(const unsigned char *buf, size_t len,
+		char *linebuf, size_t linebuflen, bool pagedump,
+		unsigned int prefix)
+{
+	static const char hex_asc[] = "0123456789abcdef";
+	unsigned char ch;
+	int j, lx = 0;
+	int ascii_column;
+
+	if (pagedump)
+		sprintf(linebuf, "0x%.8x: ", prefix);
+	else
+		sprintf(linebuf, "  OOB Data: ");
+	lx += 12;
+
+	if (!len)
+		goto nil;
+	if (len > PRETTY_ROW_SIZE)	/* limit to one line at a time */
+		len = PRETTY_ROW_SIZE;
+
+	for (j = 0; (j < len) && (lx + 3) <= linebuflen; j++) {
+		ch = buf[j];
+		linebuf[lx++] = hex_asc[ch & 0x0f];
+		linebuf[lx++] = hex_asc[(ch & 0xf0) >> 4];
+		linebuf[lx++] = ' ';
+	}
+	if (j)
+		lx--;
+
+	ascii_column = 3 * PRETTY_ROW_SIZE + 14;
+
+	if (!ascii)
+		goto nil;
+
+	while (lx < (linebuflen - 1) && lx < (ascii_column - 1))
+		linebuf[lx++] = ' ';
+	linebuf[lx++] = '|';
+	for (j = 0; (j < len) && (lx + 2) < linebuflen; j++)
+		linebuf[lx++] = (isascii(buf[j]) && isprint(buf[j])) ? buf[j]
+			: '.';
+	linebuf[lx++] = '|';
+nil:
+	linebuf[lx++] = '\n';
+	linebuf[lx++] = '\0';
+}
+
 /*
  * Buffers for reading data from flash
  */
@@ -189,7 +263,7 @@ int main(int argc, char * const argv[])
 	int ret, i, fd, ofd, bs, badblock = 0;
 	struct mtd_oob_buf oob = {0, 16, oobbuf};
 	mtd_info_t meminfo;
-	char pretty_buf[80];
+	char pretty_buf[PRETTY_BUF_LEN];
 	int oobinfochanged = 0 ;
 	struct nand_oobinfo old_oobinfo;
 	struct mtd_ecc_stats stat1, stat2;
@@ -336,20 +410,10 @@ int main(int argc, char * const argv[])
 
 		/* Write out page data */
 		if (pretty_print) {
-			for (i = 0; i < bs; i += 16) {
-				sprintf(pretty_buf,
-						"0x%08x: %02x %02x %02x %02x %02x %02x %02x "
-						"%02x %02x %02x %02x %02x %02x %02x %02x %02x\n",
-						(unsigned int) (ofs + i),  readbuf[i],
-						readbuf[i+1], readbuf[i+2],
-						readbuf[i+3], readbuf[i+4],
-						readbuf[i+5], readbuf[i+6],
-						readbuf[i+7], readbuf[i+8],
-						readbuf[i+9], readbuf[i+10],
-						readbuf[i+11], readbuf[i+12],
-						readbuf[i+13], readbuf[i+14],
-						readbuf[i+15]);
-				write(ofd, pretty_buf, 60);
+			for (i = 0; i < bs; i += PRETTY_ROW_SIZE) {
+				pretty_dump_to_buffer(readbuf+i, PRETTY_ROW_SIZE,
+						pretty_buf, PRETTY_BUF_LEN, true, ofs+i);
+				write(ofd, pretty_buf, strlen(pretty_buf));
 			}
 		} else
 			write(ofd, readbuf, bs);
@@ -372,26 +436,10 @@ int main(int argc, char * const argv[])
 
 		/* Write out OOB data */
 		if (pretty_print) {
-			if (meminfo.oobsize < 16) {
-				sprintf(pretty_buf, "  OOB Data: %02x %02x %02x %02x %02x %02x "
-						"%02x %02x\n",
-						oobbuf[0], oobbuf[1], oobbuf[2],
-						oobbuf[3], oobbuf[4], oobbuf[5],
-						oobbuf[6], oobbuf[7]);
-				write(ofd, pretty_buf, 48);
-				continue;
-			}
-
 			for (i = 0; i < meminfo.oobsize; i += 16) {
-				sprintf(pretty_buf, "  OOB Data: %02x %02x %02x %02x %02x %02x "
-						"%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n",
-						oobbuf[i], oobbuf[i+1], oobbuf[i+2],
-						oobbuf[i+3], oobbuf[i+4], oobbuf[i+5],
-						oobbuf[i+6], oobbuf[i+7], oobbuf[i+8],
-						oobbuf[i+9], oobbuf[i+10], oobbuf[i+11],
-						oobbuf[i+12], oobbuf[i+13], oobbuf[i+14],
-						oobbuf[i+15]);
-				write(ofd, pretty_buf, 60);
+				pretty_dump_to_buffer(oobbuf+i, meminfo.oobsize-i,
+						pretty_buf, PRETTY_BUF_LEN, false, 0);
+				write(ofd, pretty_buf, strlen(pretty_buf));
 			}
 		} else
 			write(ofd, oobbuf, meminfo.oobsize);
-- 
1.7.0.4

Re: [PATCH 2/6] mtd-utils/nanddump.c: Robust pretty hexdump

[Artem Bityutskiy]

On Thu, 2010-07-08 at 13:50 -0700, Brian Norris wrote:
> Adapted code from the linux kernel hex_dump_to_buffer() (lib/hexdump.c)
> to provide a more robust hexdump for the pretty option. Now, nanddump
> can print out any size of OOB (or page for that matter...) without
> having to worry about non-multiples of 16.
> 
> This also provides ability to dump ASCII format next to the hex output
> once additional command-line flags are added.
> 
> Tested with Samsung K9GAG08U0D
> 
> Signed-off-by: Brian Norris <norris@broadcom.com>

Could you please make this function completely independent? Later, if
needed, it should be possible to make it a library function and make
other mtd utils use it as well.

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

[PATCH 3/6] mtd-utils/nanddump.c: Add canonical (hex+ascii) flag

[Brian Norris]

Added the "-c" or "--canonicalprint" flag (modelled off 'hexdump -C')
so that users can choose to print ASCII output next to the prettyprint
output. This is really an extension to the prettyprint option, so the two
options do not conflict if they are both included in the same execution.

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

diff --git a/nanddump.c b/nanddump.c
index 7281279..2e316b9 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -45,6 +45,7 @@ static void display_help (void)
 "\n"
 "           --help               Display this help and exit\n"
 "           --version            Output version information and exit\n"
+"-c         --canonicalprint     Print canonical Hex+ASCII dump\n"
 "-f file    --file=file          Dump to file\n"
 "-i         --ignoreerrors       Ignore errors\n"
 "-l length  --length=length      Length\n"
@@ -74,7 +75,7 @@ static void display_version (void)
 // Option variables
 
 static bool		ignoreerrors = false;	// ignore errors
-static bool		pretty_print = false;	// print nice in ascii
+static bool		pretty_print = false;	// print nice
 static bool		noecc = false;		// don't error correct
 static bool		omitoob = false;	// omit oob data
 static unsigned long	start_addr;		// start address
@@ -83,7 +84,7 @@ static const char	*mtddev;		// mtd device name
 static const char	*dumpfile;		// dump file name
 static bool		omitbad = false;
 static bool		quiet = false;		// suppress diagnostic output
-static bool		ascii = false;
+static bool		ascii = false;		// print nice + ascii
 
 static void process_options (int argc, char * const argv[])
 {
@@ -91,10 +92,11 @@ static void process_options (int argc, char * const argv[])
 
 	for (;;) {
 		int option_index = 0;
-		static const char *short_options = "bs:f:il:opqn";
+		static const char *short_options = "bs:f:il:opqnc";
 		static const struct option long_options[] = {
 			{"help", no_argument, 0, 0},
 			{"version", no_argument, 0, 0},
+			{"canonicalprint", no_argument, 0, 'c'},
 			{"file", required_argument, 0, 'f'},
 			{"ignoreerrors", no_argument, 0, 'i'},
 			{"prettyprint", no_argument, 0, 'p'},
@@ -145,6 +147,8 @@ static void process_options (int argc, char * const argv[])
 			case 'o':
 				omitoob = true;
 				break;
+			case 'c':
+				ascii = true;
 			case 'p':
 				pretty_print = true;
 				break;
-- 
1.7.0.4

[PATCH 4/6] mtd-utils/mkfs.jffs2: fixed warnings

[Brian Norris]

Changed "char*" to "const char*" in certain function argument lists
to prevent compiler warnings for passing a hard-coded string.

Signed-off-by: Brian Norris <norris@broadcom.com>
---
 mkfs.jffs2.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/mkfs.jffs2.c b/mkfs.jffs2.c
index 64cafb6..bfa1c6e 100644
--- a/mkfs.jffs2.c
+++ b/mkfs.jffs2.c
@@ -309,8 +309,8 @@ static struct filesystem_entry *find_filesystem_entry(
 	return (NULL);
 }
 
-static struct filesystem_entry *add_host_filesystem_entry(
-		char *name, char *path, unsigned long uid, unsigned long gid,
+static struct filesystem_entry *add_host_filesystem_entry(const char *name,
+		const char *path, unsigned long uid, unsigned long gid,
 		unsigned long mode, dev_t rdev, struct filesystem_entry *parent)
 {
 	int status;
@@ -401,7 +401,8 @@ static struct filesystem_entry *add_host_filesystem_entry(
 }
 
 static struct filesystem_entry *recursive_add_host_directory(
-		struct filesystem_entry *parent, char *targetpath, char *hostpath)
+		struct filesystem_entry *parent, const char *targetpath,
+		const char *hostpath)
 {
 	int i, n;
 	struct stat sb;
-- 
1.7.0.4

Re: [PATCH 4/6] mtd-utils/mkfs.jffs2: fixed warnings

[Artem Bityutskiy]

On Thu, 2010-07-08 at 13:50 -0700, Brian Norris wrote:
> Changed "char*" to "const char*" in certain function argument lists
> to prevent compiler warnings for passing a hard-coded string.
> 
> Signed-off-by: Brian Norris <norris@broadcom.com>

Pushed, thanks.

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

[PATCH 5/6] mtd-utils/nandtest.c: Fixed indentation

[Brian Norris]

Replaced tabs with spaces in the help message for nandtest to
fix problems with varying indentation and to provide consistency
with other utils in the set.

Signed-off-by: Brian Norris <norris@broadcom.com>
---
 nandtest.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/nandtest.c b/nandtest.c
index e31e28a..13e9577 100644
--- a/nandtest.c
+++ b/nandtest.c
@@ -18,13 +18,13 @@
 void usage(void)
 {
 	fprintf(stderr, "usage: nandtest [OPTIONS] <device>\n\n"
-		"  -h, --help		Display this help output\n"
-		"  -m, --markbad	Mark blocks bad if they appear so\n"
-		"  -s, --seed		Supply random seed\n"
-		"  -p, --passes		Number of passes\n"
-		"  -o, --offset		Start offset on flash\n"
-		"  -l, --length		Length of flash to test\n"
-		"  -k, --keep		Restore existing contents after test\n");
+		"  -h, --help           Display this help output\n"
+		"  -m, --markbad        Mark blocks bad if they appear so\n"
+		"  -s, --seed           Supply random seed\n"
+		"  -p, --passes         Number of passes\n"
+		"  -o, --offset         Start offset on flash\n"
+		"  -l, --length         Length of flash to test\n"
+		"  -k, --keep           Restore existing contents after test\n");
 	exit(1);
 }
 
-- 
1.7.0.4

Re: [PATCH 5/6] mtd-utils/nandtest.c: Fixed indentation

[Artem Bityutskiy]

On Thu, 2010-07-08 at 13:50 -0700, Brian Norris wrote:
> Replaced tabs with spaces in the help message for nandtest to
> fix problems with varying indentation and to provide consistency
> with other utils in the set.
> 
> Signed-off-by: Brian Norris <norris@broadcom.com>

Pushed, thanks!

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

[PATCH 6/6] mtd-utils/nanddump.c: Add "forcebinary" flag

[Brian Norris]

Restrict binary dumping so that by default, binary garbage is not
printed directly to a terminal. Output redicted to files or piped to
other commands should not be affected (as judged by "isatty(ofd)").
A new flag "-a" or "--forcebinary" is included so that users can
override this behavior if necessary.

Signed-off-by: Brian Norris <norris@broadcom.com>
---
 nanddump.c |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index 2e316b9..8815bdb 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -45,6 +45,7 @@ static void display_help (void)
 "\n"
 "           --help               Display this help and exit\n"
 "           --version            Output version information and exit\n"
+"-a         --forcebinary        Force printing of binary data to tty\n"
 "-c         --canonicalprint     Print canonical Hex+ASCII dump\n"
 "-f file    --file=file          Dump to file\n"
 "-i         --ignoreerrors       Ignore errors\n"
@@ -85,6 +86,7 @@ static const char	*dumpfile;		// dump file name
 static bool		omitbad = false;
 static bool		quiet = false;		// suppress diagnostic output
 static bool		ascii = false;		// print nice + ascii
+static bool		forcebinary = false;	// force printing binary to tty
 
 static void process_options (int argc, char * const argv[])
 {
@@ -92,10 +94,11 @@ static void process_options (int argc, char * const argv[])
 
 	for (;;) {
 		int option_index = 0;
-		static const char *short_options = "bs:f:il:opqnc";
+		static const char *short_options = "bs:f:il:opqnca";
 		static const struct option long_options[] = {
 			{"help", no_argument, 0, 0},
 			{"version", no_argument, 0, 0},
+			{"forcebinary", no_argument, 0, 'a'},
 			{"canonicalprint", no_argument, 0, 'c'},
 			{"file", required_argument, 0, 'f'},
 			{"ignoreerrors", no_argument, 0, 'i'},
@@ -147,6 +150,9 @@ static void process_options (int argc, char * const argv[])
 			case 'o':
 				omitoob = true;
 				break;
+			case 'a':
+				forcebinary = true;
+				break;
 			case 'c':
 				ascii = true;
 			case 'p':
@@ -170,6 +176,13 @@ static void process_options (int argc, char * const argv[])
 		exit(EXIT_FAILURE);
 	}
 
+	if (forcebinary && pretty_print) {
+		fprintf(stderr, "The forcebinary and pretty print options are\n"
+				"mutually-exclusive. Choose one or the "
+				"other.\n");
+		exit(EXIT_FAILURE);
+	}
+
 	if ((argc - optind) != 1 || error)
 		display_help ();
 
@@ -354,6 +367,13 @@ int main(int argc, char * const argv[])
 		exit(EXIT_FAILURE);
 	}
 
+	if (!pretty_print && !forcebinary && isatty(ofd)) {
+		fprintf(stderr, "Not printing binary garbage to tty. Use '-a'\n"
+				"or '--forcebinary' to override.\n");
+		close(fd);
+		exit(EXIT_FAILURE);
+	}
+
 	/* Initialize start/end addresses and block size */
 	if (length)
 		end_addr = start_addr + length;
-- 
1.7.0.4

Re: [PATCH 6/6] mtd-utils/nanddump.c: Add "forcebinary" flag

[Brian Norris]

Sorry, apparently some of the "In-Reply-To" information got screwed up. 
The cover e-mail as well as patches 1 and 3 are in the proper chain, but 
not 2, 4, 5, 6. :(

Brian

Re: [PATCH 6/6] mtd-utils/nanddump.c: Add "forcebinary" flag

[Artem Bityutskiy]

On Thu, 2010-07-08 at 13:50 -0700, Brian Norris wrote:
> Restrict binary dumping so that by default, binary garbage is not
> printed directly to a terminal. Output redicted to files or piped to
> other commands should not be affected (as judged by "isatty(ofd)").
> A new flag "-a" or "--forcebinary" is included so that users can
> override this behavior if necessary.
> 
> Signed-off-by: Brian Norris <norris@broadcom.com>

Could you pleas make hexdump independent (make the ascii switch to be
the function argument, not a global knob), and re-send the nanddump
patches? I did not push patches 2, 3, 6, but pushed the rest of the
patches.

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

[PATCH v2 2/6] mtd-utils/nanddump.c: Robust pretty hexdump

[Brian Norris]

These should be what you were looking for.

Adapted code from the linux kernel hex_dump_to_buffer() (lib/hexdump.c)
to provide a more robust hexdump for the pretty option. Now, nanddump
can print out any size of OOB (or page for that matter...) without
having to worry about non-multiples of 16.

This also provides ability to dump ASCII format next to the hex output
once additional command-line flags are added.

Tested with Samsung K9GAG08U0D

Signed-off-by: Brian Norris <norris@broadcom.com>
---
 nanddump.c |  117 ++++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 83 insertions(+), 34 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index 735ae48..eccb651 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -83,6 +83,7 @@ static const char	*mtddev;		// mtd device name
 static const char	*dumpfile;		// dump file name
 static bool		omitbad = false;
 static bool		quiet = false;		// suppress diagnostic output
+static bool		canonical = false;
 
 static void process_options (int argc, char * const argv[])
 {
@@ -171,6 +172,80 @@ static void process_options (int argc, char * const argv[])
 	mtddev = argv[optind];
 }
 
+#define PRETTY_ROW_SIZE 16
+#define PRETTY_BUF_LEN 80
+
+/**
+ * pretty_dump_to_buffer - formats a blob of data to "hex ASCII" in memory
+ * @buf: data blob to dump
+ * @len: number of bytes in the @buf
+ * @linebuf: where to put the converted data
+ * @linebuflen: total size of @linebuf, including space for terminating NULL
+ * @pagedump: true - dumping as page format; false - dumping as OOB format
+ * @ascii: dump ascii formatted data next to hexdump
+ * @prefix: address to print before line in a page dump, ignored if !pagedump
+ *
+ * pretty_dump_to_buffer() works on one "line" of output at a time, i.e.,
+ * PRETTY_ROW_SIZE bytes of input data converted to hex + ASCII output.
+ *
+ * Given a buffer of unsigned char data, pretty_dump_to_buffer() converts the
+ * input data to a hex/ASCII dump at the supplied memory location. A prefix
+ * is included based on whether we are dumping page or OOB data. The converted
+ * output is always NULL-terminated.
+ *
+ * e.g.
+ *   pretty_dump_to_buffer(data, data_len, prettybuf, linelen, true,
+ *                         false, 256);
+ * produces:
+ *   0x00000100: 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f
+ * NOTE: This function was adapted from linux kernel, "lib/hexdump.c"
+ */
+static void pretty_dump_to_buffer(const unsigned char *buf, size_t len,
+		char *linebuf, size_t linebuflen, bool pagedump, bool ascii,
+		unsigned int prefix)
+{
+	static const char hex_asc[] = "0123456789abcdef";
+	unsigned char ch;
+	int j, lx = 0;
+	int ascii_column;
+
+	if (pagedump)
+		sprintf(linebuf, "0x%.8x: ", prefix);
+	else
+		sprintf(linebuf, "  OOB Data: ");
+	lx += 12;
+
+	if (!len)
+		goto nil;
+	if (len > PRETTY_ROW_SIZE)	/* limit to one line at a time */
+		len = PRETTY_ROW_SIZE;
+
+	for (j = 0; (j < len) && (lx + 3) <= linebuflen; j++) {
+		ch = buf[j];
+		linebuf[lx++] = hex_asc[ch & 0x0f];
+		linebuf[lx++] = hex_asc[(ch & 0xf0) >> 4];
+		linebuf[lx++] = ' ';
+	}
+	if (j)
+		lx--;
+
+	ascii_column = 3 * PRETTY_ROW_SIZE + 14;
+
+	if (!ascii)
+		goto nil;
+
+	while (lx < (linebuflen - 1) && lx < (ascii_column - 1))
+		linebuf[lx++] = ' ';
+	linebuf[lx++] = '|';
+	for (j = 0; (j < len) && (lx + 2) < linebuflen; j++)
+		linebuf[lx++] = (isascii(buf[j]) && isprint(buf[j])) ? buf[j]
+			: '.';
+	linebuf[lx++] = '|';
+nil:
+	linebuf[lx++] = '\n';
+	linebuf[lx++] = '\0';
+}
+
 /*
  * Buffers for reading data from flash
  */
@@ -189,7 +264,7 @@ int main(int argc, char * const argv[])
 	int ret, i, fd, ofd, bs, badblock = 0;
 	struct mtd_oob_buf oob = {0, 16, oobbuf};
 	mtd_info_t meminfo;
-	char pretty_buf[80];
+	char pretty_buf[PRETTY_BUF_LEN];
 	int oobinfochanged = 0 ;
 	struct nand_oobinfo old_oobinfo;
 	struct mtd_ecc_stats stat1, stat2;
@@ -336,20 +411,10 @@ int main(int argc, char * const argv[])
 
 		/* Write out page data */
 		if (pretty_print) {
-			for (i = 0; i < bs; i += 16) {
-				sprintf(pretty_buf,
-						"0x%08x: %02x %02x %02x %02x %02x %02x %02x "
-						"%02x %02x %02x %02x %02x %02x %02x %02x %02x\n",
-						(unsigned int) (ofs + i),  readbuf[i],
-						readbuf[i+1], readbuf[i+2],
-						readbuf[i+3], readbuf[i+4],
-						readbuf[i+5], readbuf[i+6],
-						readbuf[i+7], readbuf[i+8],
-						readbuf[i+9], readbuf[i+10],
-						readbuf[i+11], readbuf[i+12],
-						readbuf[i+13], readbuf[i+14],
-						readbuf[i+15]);
-				write(ofd, pretty_buf, 60);
+			for (i = 0; i < bs; i += PRETTY_ROW_SIZE) {
+				pretty_dump_to_buffer(readbuf+i, PRETTY_ROW_SIZE,
+						pretty_buf, PRETTY_BUF_LEN, true, canonical, ofs+i);
+				write(ofd, pretty_buf, strlen(pretty_buf));
 			}
 		} else
 			write(ofd, readbuf, bs);
@@ -372,26 +437,10 @@ int main(int argc, char * const argv[])
 
 		/* Write out OOB data */
 		if (pretty_print) {
-			if (meminfo.oobsize < 16) {
-				sprintf(pretty_buf, "  OOB Data: %02x %02x %02x %02x %02x %02x "
-						"%02x %02x\n",
-						oobbuf[0], oobbuf[1], oobbuf[2],
-						oobbuf[3], oobbuf[4], oobbuf[5],
-						oobbuf[6], oobbuf[7]);
-				write(ofd, pretty_buf, 48);
-				continue;
-			}
-
 			for (i = 0; i < meminfo.oobsize; i += 16) {
-				sprintf(pretty_buf, "  OOB Data: %02x %02x %02x %02x %02x %02x "
-						"%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n",
-						oobbuf[i], oobbuf[i+1], oobbuf[i+2],
-						oobbuf[i+3], oobbuf[i+4], oobbuf[i+5],
-						oobbuf[i+6], oobbuf[i+7], oobbuf[i+8],
-						oobbuf[i+9], oobbuf[i+10], oobbuf[i+11],
-						oobbuf[i+12], oobbuf[i+13], oobbuf[i+14],
-						oobbuf[i+15]);
-				write(ofd, pretty_buf, 60);
+				pretty_dump_to_buffer(oobbuf+i, meminfo.oobsize-i,
+						pretty_buf, PRETTY_BUF_LEN, false, canonical, 0);
+				write(ofd, pretty_buf, strlen(pretty_buf));
 			}
 		} else
 			write(ofd, oobbuf, meminfo.oobsize);
-- 
1.7.0.4

Re: [PATCH v2 2/6] mtd-utils/nanddump.c: Robust pretty hexdump

[Artem Bityutskiy]

On Mon, 2010-07-19 at 10:33 -0700, Brian Norris wrote:
> These should be what you were looking for.
> 
> Adapted code from the linux kernel hex_dump_to_buffer() (lib/hexdump.c)
> to provide a more robust hexdump for the pretty option. Now, nanddump
> can print out any size of OOB (or page for that matter...) without
> having to worry about non-multiples of 16.
> 
> This also provides ability to dump ASCII format next to the hex output
> once additional command-line flags are added.
> 
> Tested with Samsung K9GAG08U0D
> 
> Signed-off-by: Brian Norris <norris@broadcom.com>

Pushed all 3 to mtd-utils, thank you!

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

[PATCH v2 3/6] mtd-utils/nanddump.c: Add canonical (hex+ascii) flag

[Brian Norris]

Added the "-c" or "--canonicalprint" flag (modelled off 'hexdump -C')
so that users can choose to print ASCII output next to the prettyprint
output. This is really an extension to the prettyprint option, so the two
options do not conflict if they are both included in the same execution.

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

diff --git a/nanddump.c b/nanddump.c
index eccb651..cd018c6 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -45,6 +45,7 @@ static void display_help (void)
 "\n"
 "           --help               Display this help and exit\n"
 "           --version            Output version information and exit\n"
+"-c         --canonicalprint     Print canonical Hex+ASCII dump\n"
 "-f file    --file=file          Dump to file\n"
 "-i         --ignoreerrors       Ignore errors\n"
 "-l length  --length=length      Length\n"
@@ -74,7 +75,7 @@ static void display_version (void)
 // Option variables
 
 static bool		ignoreerrors = false;	// ignore errors
-static bool		pretty_print = false;	// print nice in ascii
+static bool		pretty_print = false;	// print nice
 static bool		noecc = false;		// don't error correct
 static bool		omitoob = false;	// omit oob data
 static unsigned long	start_addr;		// start address
@@ -83,7 +84,7 @@ static const char	*mtddev;		// mtd device name
 static const char	*dumpfile;		// dump file name
 static bool		omitbad = false;
 static bool		quiet = false;		// suppress diagnostic output
-static bool		canonical = false;
+static bool		canonical = false;	// print nice + ascii
 
 static void process_options (int argc, char * const argv[])
 {
@@ -91,10 +92,11 @@ static void process_options (int argc, char * const argv[])
 
 	for (;;) {
 		int option_index = 0;
-		static const char *short_options = "bs:f:il:opqn";
+		static const char *short_options = "bs:f:il:opqnc";
 		static const struct option long_options[] = {
 			{"help", no_argument, 0, 0},
 			{"version", no_argument, 0, 0},
+			{"canonicalprint", no_argument, 0, 'c'},
 			{"file", required_argument, 0, 'f'},
 			{"ignoreerrors", no_argument, 0, 'i'},
 			{"prettyprint", no_argument, 0, 'p'},
@@ -145,6 +147,8 @@ static void process_options (int argc, char * const argv[])
 			case 'o':
 				omitoob = true;
 				break;
+			case 'c':
+				canonical = true;
 			case 'p':
 				pretty_print = true;
 				break;
-- 
1.7.0.4

[PATCH v2 6/6] mtd-utils/nanddump.c: Add "forcebinary" flag

[Brian Norris]

Restrict binary dumping so that by default, binary garbage is not
printed directly to a terminal. Output redicted to files or piped to
other commands should not be affected (as judged by "isatty(ofd)").
A new flag "-a" or "--forcebinary" is included so that users can
override this behavior if necessary.

Signed-off-by: Brian Norris <norris@broadcom.com>
---
 nanddump.c |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index cd018c6..acfdb33 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -45,6 +45,7 @@ static void display_help (void)
 "\n"
 "           --help               Display this help and exit\n"
 "           --version            Output version information and exit\n"
+"-a         --forcebinary        Force printing of binary data to tty\n"
 "-c         --canonicalprint     Print canonical Hex+ASCII dump\n"
 "-f file    --file=file          Dump to file\n"
 "-i         --ignoreerrors       Ignore errors\n"
@@ -85,6 +86,7 @@ static const char	*dumpfile;		// dump file name
 static bool		omitbad = false;
 static bool		quiet = false;		// suppress diagnostic output
 static bool		canonical = false;	// print nice + ascii
+static bool		forcebinary = false;	// force printing binary to tty
 
 static void process_options (int argc, char * const argv[])
 {
@@ -92,10 +94,11 @@ static void process_options (int argc, char * const argv[])
 
 	for (;;) {
 		int option_index = 0;
-		static const char *short_options = "bs:f:il:opqnc";
+		static const char *short_options = "bs:f:il:opqnca";
 		static const struct option long_options[] = {
 			{"help", no_argument, 0, 0},
 			{"version", no_argument, 0, 0},
+			{"forcebinary", no_argument, 0, 'a'},
 			{"canonicalprint", no_argument, 0, 'c'},
 			{"file", required_argument, 0, 'f'},
 			{"ignoreerrors", no_argument, 0, 'i'},
@@ -147,6 +150,9 @@ static void process_options (int argc, char * const argv[])
 			case 'o':
 				omitoob = true;
 				break;
+			case 'a':
+				forcebinary = true;
+				break;
 			case 'c':
 				canonical = true;
 			case 'p':
@@ -170,6 +176,13 @@ static void process_options (int argc, char * const argv[])
 		exit(EXIT_FAILURE);
 	}
 
+	if (forcebinary && pretty_print) {
+		fprintf(stderr, "The forcebinary and pretty print options are\n"
+				"mutually-exclusive. Choose one or the "
+				"other.\n");
+		exit(EXIT_FAILURE);
+	}
+
 	if ((argc - optind) != 1 || error)
 		display_help ();
 
@@ -355,6 +368,13 @@ int main(int argc, char * const argv[])
 		exit(EXIT_FAILURE);
 	}
 
+	if (!pretty_print && !forcebinary && isatty(ofd)) {
+		fprintf(stderr, "Not printing binary garbage to tty. Use '-a'\n"
+				"or '--forcebinary' to override.\n");
+		close(fd);
+		exit(EXIT_FAILURE);
+	}
+
 	/* Initialize start/end addresses and block size */
 	if (length)
 		end_addr = start_addr + length;
-- 
1.7.0.4