mtd-utils: formatting of odd-sized OOB in nanddump

PATCH In review Brian Norris 2 participants 21 messages Jun 16, 2010 – Jul 21, 2010 Memory technology devices (MTD)

Share & tools

Brian Norris

author

2010-06-16 16:42 UTC #0 lore raw

PATCH 1/1 mtd-utils: formatting of odd-sized OOB in nanddump

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 <[email protected]>

---
 nanddump.c |   31 ++++++++++++++++++++++---------
 1 files changed, 22 insertions(+), 9 deletions(-)

nanddump.c +22 -9

index e44ab36..190a653 100644
@@ -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);

Signature · 1 line

1.7.1

Artem Bityutskiy

2010-07-07 14:11 UTC #1 lore raw

Quoted 11 lines · Brian Norris

nanddump.c +11 -0

+				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);

nanddump.c int main(int argc, char * const argv[]) · line 405

Quoted 10 lines

nanddump.c +10 -0

+				} 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 ...

Signature · 2 lines

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

Brian Norris

author

2010-07-07 18:22 UTC #2 lore raw

Quoted 1 line

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? :)

Quoted 2 lines

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

Artem Bityutskiy

2010-07-08 04:09 UTC #3 lore raw

Hi,

Quoted 9 lines · Brian Norris

Quoted 1 line
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.

Quoted 11 lines

Quoted 3 lines
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.

Signature · 2 lines

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

Brian Norris

author

2010-07-08 20:52 UTC #4 lore raw

PATCH 1/1 mtd-utils update

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

Brian Norris

author

2010-07-08 20:52 UTC #5 lore raw

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

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 <[email protected]>

---
 nanddump.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

nanddump.c +4 -2

index e44ab36..735ae48 100644
@@ -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

Signature · 1 line

1.7.0.4

Artem Bityutskiy

Applied

2010-07-18 07:17 UTC #6 lore raw

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

Quoted 5 lines · 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 <[email protected]>

Pushed to mtd-utils.git

Signature · 2 lines

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

Brian Norris

author

2010-07-08 20:51 UTC #7 lore raw

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

In reply to 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 <[email protected]>

---
 nanddump.c |  116 ++++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 82 insertions(+), 34 deletions(-)

nanddump.c +82 ~~-34~~

index 735ae48..7281279 100644
@@ -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);

Signature · 1 line

1.7.0.4

Artem Bityutskiy

2010-07-18 07:18 UTC #8 lore raw

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

Quoted 11 lines · 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 <[email protected]>

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.

Signature · 2 lines

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

Brian Norris

author

2010-07-08 20:52 UTC #9 lore raw

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

In reply to 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 <[email protected]>

---
 nanddump.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

nanddump.c +7 -3

index 7281279..2e316b9 100644
@@ -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;

Signature · 1 line

1.7.0.4

Brian Norris

author

2010-07-08 20:51 UTC #10 lore raw

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

In reply to 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 <[email protected]>

---
 mkfs.jffs2.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

mkfs.jffs2.c +4 -3

index 64cafb6..bfa1c6e 100644
@@ -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;

Signature · 1 line

1.7.0.4

Artem Bityutskiy

2010-07-18 07:19 UTC #11 lore raw

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

Quoted 4 lines · 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 <[email protected]>

Pushed, thanks.

Signature · 2 lines

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

Brian Norris

author

2010-07-08 20:51 UTC #12 lore raw

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

In reply to 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 <[email protected]>

---
 nandtest.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

nandtest.c +7 -7

index e31e28a..13e9577 100644
@@ -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);
 }

Signature · 1 line

1.7.0.4

Artem Bityutskiy

2010-07-18 07:21 UTC #13 lore raw

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

Quoted 5 lines · 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 <[email protected]>

Pushed, thanks!

Signature · 2 lines

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

Brian Norris

author

2010-07-08 20:51 UTC #14 lore raw

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

In reply to 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 <[email protected]>

---
 nanddump.c |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)

nanddump.c +21 -1

index 2e316b9..8815bdb 100644
@@ -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;

Signature · 1 line

1.7.0.4

Brian Norris

author

2010-07-08 22:03 UTC #15 lore raw

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

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

Artem Bityutskiy

2010-07-18 07:23 UTC #16 lore raw

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

In reply to Brian Norris

Quoted 7 lines · 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 <[email protected]>

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.

Signature · 2 lines

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

Brian Norris

author

2010-07-19 17:33 UTC #17 lore raw

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

These should be what you were looking for.

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 <[email protected]>

---
 nanddump.c |  117 ++++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 83 insertions(+), 34 deletions(-)

nanddump.c +83 ~~-34~~

index 735ae48..eccb651 100644
@@ -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);

Signature · 1 line

1.7.0.4

Artem Bityutskiy

2010-07-21 10:00 UTC #18 lore raw

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

Quoted 7 lines · 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.

Quoted 6 lines

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 <[email protected]>

Pushed all 3 to mtd-utils, thank you!

Signature · 2 lines

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

Brian Norris

author

2010-07-19 17:33 UTC #19 lore raw

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

In reply to Artem Bityutskiy

Signed-off-by Brian Norris <[email protected]>

---
 nanddump.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

nanddump.c +7 -3

index eccb651..cd018c6 100644
@@ -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;

Signature · 1 line

1.7.0.4

Brian Norris

author

2010-07-19 17:33 UTC #20 lore raw

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

In reply to Artem Bityutskiy

Signed-off-by Brian Norris <[email protected]>

---
 nanddump.c |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)

nanddump.c +21 -1

index cd018c6..acfdb33 100644
@@ -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;

Signature · 1 line

1.7.0.4