[PATCH 1/2] nanddump: drop unused --ignoreerrors option

[PATCH 1/2] nanddump: drop unused --ignoreerrors option

[Mike Frysinger]

Nowhere in the nanddump code is the "ignoreerrors" variable used.  So
drop the option completely.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 nanddump.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index 22fec5f..70b78f0 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -48,7 +48,6 @@ static void display_help (void)
 "-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"
 "-l length  --length=length      Length\n"
 "-n         --noecc              Read without error correction\n"
 "-o         --omitoob            Omit oob data\n"
@@ -75,7 +74,6 @@ static void display_version (void)
 
 // Option variables
 
-static bool		ignoreerrors = false;	// ignore errors
 static bool		pretty_print = false;	// print nice
 static bool		noecc = false;		// don't error correct
 static bool		omitoob = false;	// omit oob data
@@ -94,14 +92,13 @@ static void process_options (int argc, char * const argv[])
 
 	for (;;) {
 		int option_index = 0;
-		static const char *short_options = "bs:f:il:opqnca";
+		static const char *short_options = "bs:f:l: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'},
 			{"prettyprint", no_argument, 0, 'p'},
 			{"omitoob", no_argument, 0, 'o'},
 			{"omitbad", no_argument, 0, 'b'},
@@ -141,9 +138,6 @@ static void process_options (int argc, char * const argv[])
 					exit(EXIT_FAILURE);
 				}
 				break;
-			case 'i':
-				ignoreerrors = true;
-				break;
 			case 'l':
 				length = strtol(optarg, NULL, 0);
 				break;
-- 
1.7.2

[PATCH 2/2] nanddump: add --nobad to read bad blocks

[Mike Frysinger]

Sometimes dumping bad blocks is useful, like when the data isn't actually
bad but the OOB layout isn't what the kernel is expecting or is otherwise
screwed up.  The --nobad option allows just that.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 nanddump.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index 70b78f0..8b3d4a1 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -50,6 +50,7 @@ static void display_help (void)
 "-f file    --file=file          Dump to file\n"
 "-l length  --length=length      Length\n"
 "-n         --noecc              Read without error correction\n"
+"-N         --nobad              Read without bad block skipping\n"
 "-o         --omitoob            Omit oob data\n"
 "-b         --omitbad            Omit bad blocks from the dump\n"
 "-p         --prettyprint        Print nice (hexdump)\n"
@@ -76,6 +77,7 @@ static void display_version (void)
 
 static bool		pretty_print = false;	// print nice
 static bool		noecc = false;		// don't error correct
+static bool		nobad = false;		// don't skip bad blocks
 static bool		omitoob = false;	// omit oob data
 static unsigned long	start_addr;		// start address
 static unsigned long	length;			// dump length
@@ -92,7 +94,7 @@ static void process_options (int argc, char * const argv[])
 
 	for (;;) {
 		int option_index = 0;
-		static const char *short_options = "bs:f:l:opqnca";
+		static const char *short_options = "bs:f:l:opqnNca";
 		static const struct option long_options[] = {
 			{"help", no_argument, 0, 0},
 			{"version", no_argument, 0, 0},
@@ -105,6 +107,7 @@ static void process_options (int argc, char * const argv[])
 			{"startaddress", required_argument, 0, 's'},
 			{"length", required_argument, 0, 'l'},
 			{"noecc", no_argument, 0, 'n'},
+			{"nobad", no_argument, 0, 'N'},
 			{"quiet", no_argument, 0, 'q'},
 			{0, 0, 0, 0},
 		};
@@ -158,6 +161,9 @@ static void process_options (int argc, char * const argv[])
 			case 'n':
 				noecc = true;
 				break;
+			case 'N':
+				nobad = true;
+				break;
 			case '?':
 				error++;
 				break;
@@ -390,7 +396,9 @@ int main(int argc, char * const argv[])
 	for (ofs = start_addr; ofs < end_addr ; ofs+=bs) {
 
 		// new eraseblock , check for bad block
-		if (blockstart != (ofs & (~meminfo.erasesize + 1))) {
+		if (nobad) {
+			badblock = 0;
+		} else if (blockstart != (ofs & (~meminfo.erasesize + 1))) {
 			blockstart = ofs & (~meminfo.erasesize + 1);
 			if ((badblock = ioctl(fd, MEMGETBADBLOCK, &blockstart)) < 0) {
 				perror("ioctl(MEMGETBADBLOCK)");
-- 
1.7.2

Re: [PATCH 1/2] nanddump: drop unused --ignoreerrors option

[Artem Bityutskiy]

On Sat, 2010-09-11 at 23:50 -0400, Mike Frysinger wrote:
> Nowhere in the nanddump code is the "ignoreerrors" variable used.  So
> drop the option completely.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

Pushed, thanks.

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

Re: [PATCH 2/2] nanddump: add --nobad to read bad blocks

[Artem Bityutskiy]

On Sat, 2010-09-11 at 23:50 -0400, Mike Frysinger wrote:
> Sometimes dumping bad blocks is useful, like when the data isn't actually
> bad but the OOB layout isn't what the kernel is expecting or is otherwise
> screwed up.  The --nobad option allows just that.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  nanddump.c |   12 ++++++++++--
>  1 files changed, 10 insertions(+), 2 deletions(-)

Pushed, thanks.

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

RE: [PATCH 2/2] nanddump: add --nobad to read bad blocks

[Jon Povey]

Mike Frysinger wrote:
> Sometimes dumping bad blocks is useful, like when the data isn't
> actually
> bad but the OOB layout isn't what the kernel is expecting or is
> otherwise
> screwed up.  The --nobad option allows just that.

> +"-N         --nobad              Read without bad block skipping\n"

This doesn't seem like a good name for the option to me. A useful option but an unintuitive name, "nobad" sounds like it is going to omit bad blocks, where actually it is going to include them.

"noskipbad" or "includebad" would seem to be better.

I see this got pushed already though..

--
Jon Povey
jon.povey@racelogic.co.uk

Racelogic is a limited company registered in England. Registered number 2743719 .
Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB .

The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network

Re: [PATCH 2/2] nanddump: add --nobad to read bad blocks

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 866 bytes --]

On Mon, Sep 13, 2010 at 06:50:04AM +0100, Jon Povey wrote:
> Mike Frysinger wrote:
> > Sometimes dumping bad blocks is useful, like when the data isn't actually
> > bad but the OOB layout isn't what the kernel is expecting or is otherwise
> > screwed up.  The --nobad option allows just that.
> 
> > +"-N         --nobad              Read without bad block skipping\n"
> 
> This doesn't seem like a good name for the option to me. A useful option but
> an unintuitive name, "nobad" sounds like it is going to omit bad blocks,
> where actually it is going to include them.
> 
> "noskipbad" or "includebad" would seem to be better.

I agree. Is that still fixable despite being pushed already?

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/2] nanddump: add --nobad to read bad blocks

[Artem Bityutskiy]

On Mon, 2010-09-13 at 08:22 +0200, Wolfram Sang wrote:
> On Mon, Sep 13, 2010 at 06:50:04AM +0100, Jon Povey wrote:
> > Mike Frysinger wrote:
> > > Sometimes dumping bad blocks is useful, like when the data isn't actually
> > > bad but the OOB layout isn't what the kernel is expecting or is otherwise
> > > screwed up.  The --nobad option allows just that.
> > 
> > > +"-N         --nobad              Read without bad block skipping\n"
> > 
> > This doesn't seem like a good name for the option to me. A useful option but
> > an unintuitive name, "nobad" sounds like it is going to omit bad blocks,
> > where actually it is going to include them.
> > 
> > "noskipbad" or "includebad" would seem to be better.
> 
> I agree. Is that still fixable despite being pushed already?

Sure, send a patch please.

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

[PATCH] nanddump: Rename --nobad to --noskipbad

[Wolfram Sang]

"nobad" might lead to the assumption that bad blocks are skipped, but this
option does exactly the opposite. Use a more descriptive name.

Reported-by: Jon Povey <Jon.Povey@racelogic.co.uk>
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Mike Frysinger <vapier@gentoo.org>
---

Build tested only.

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

diff --git a/nanddump.c b/nanddump.c
index 8b3d4a1..709b2db 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -50,7 +50,7 @@ static void display_help (void)
 "-f file    --file=file          Dump to file\n"
 "-l length  --length=length      Length\n"
 "-n         --noecc              Read without error correction\n"
-"-N         --nobad              Read without bad block skipping\n"
+"-N         --noskipbad          Read without bad block skipping\n"
 "-o         --omitoob            Omit oob data\n"
 "-b         --omitbad            Omit bad blocks from the dump\n"
 "-p         --prettyprint        Print nice (hexdump)\n"
@@ -77,7 +77,7 @@ static void display_version (void)
 
 static bool		pretty_print = false;	// print nice
 static bool		noecc = false;		// don't error correct
-static bool		nobad = false;		// don't skip bad blocks
+static bool		noskipbad = false;	// don't skip bad blocks
 static bool		omitoob = false;	// omit oob data
 static unsigned long	start_addr;		// start address
 static unsigned long	length;			// dump length
@@ -107,7 +107,7 @@ static void process_options (int argc, char * const argv[])
 			{"startaddress", required_argument, 0, 's'},
 			{"length", required_argument, 0, 'l'},
 			{"noecc", no_argument, 0, 'n'},
-			{"nobad", no_argument, 0, 'N'},
+			{"noskipbad", no_argument, 0, 'N'},
 			{"quiet", no_argument, 0, 'q'},
 			{0, 0, 0, 0},
 		};
@@ -162,7 +162,7 @@ static void process_options (int argc, char * const argv[])
 				noecc = true;
 				break;
 			case 'N':
-				nobad = true;
+				noskipbad = true;
 				break;
 			case '?':
 				error++;
@@ -396,7 +396,7 @@ int main(int argc, char * const argv[])
 	for (ofs = start_addr; ofs < end_addr ; ofs+=bs) {
 
 		// new eraseblock , check for bad block
-		if (nobad) {
+		if (noskipbad) {
 			badblock = 0;
 		} else if (blockstart != (ofs & (~meminfo.erasesize + 1))) {
 			blockstart = ofs & (~meminfo.erasesize + 1);
-- 
1.7.1

Re: [PATCH] nanddump: Rename --nobad to --noskipbad

[Artem Bityutskiy]

On Mon, 2010-09-13 at 09:15 +0200, Wolfram Sang wrote:
> "nobad" might lead to the assumption that bad blocks are skipped, but this
> option does exactly the opposite. Use a more descriptive name.
> 
> Reported-by: Jon Povey <Jon.Povey@racelogic.co.uk>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Mike Frysinger <vapier@gentoo.org>

It does not apply to the latest mtd-utils tree. Would you please re-send
an updated version?

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

Re: [PATCH] nanddump: Rename --nobad to --noskipbad

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 900 bytes --]

On Fri, Sep 17, 2010 at 09:20:20AM +0300, Artem Bityutskiy wrote:
> On Mon, 2010-09-13 at 09:15 +0200, Wolfram Sang wrote:
> > "nobad" might lead to the assumption that bad blocks are skipped, but this
> > option does exactly the opposite. Use a more descriptive name.
> > 
> > Reported-by: Jon Povey <Jon.Povey@racelogic.co.uk>
> > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> > Cc: Mike Frysinger <vapier@gentoo.org>
> 
> It does not apply to the latest mtd-utils tree. Would you please re-send
> an updated version?

Ehrm, it still applies on top of Mike's original patch (it just seems
that the latter is not pushed yet despite your mail saying so). Or do
you want me to fold those two?

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] nanddump: Rename --nobad to --noskipbad

[Artem Bityutskiy]

On Fri, 2010-09-17 at 09:54 +0200, Wolfram Sang wrote:
> On Fri, Sep 17, 2010 at 09:20:20AM +0300, Artem Bityutskiy wrote:
> > On Mon, 2010-09-13 at 09:15 +0200, Wolfram Sang wrote:
> > > "nobad" might lead to the assumption that bad blocks are skipped, but this
> > > option does exactly the opposite. Use a more descriptive name.
> > > 
> > > Reported-by: Jon Povey <Jon.Povey@racelogic.co.uk>
> > > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> > > Cc: Mike Frysinger <vapier@gentoo.org>
> > 
> > It does not apply to the latest mtd-utils tree. Would you please re-send
> > an updated version?
> 
> Ehrm, it still applies on top of Mike's original patch (it just seems
> that the latter is not pushed yet despite your mail saying so). Or do
> you want me to fold those two?

Indeed forgot to push it out, sorry. No, I'll sort this out, thanks.

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

Re: [PATCH] nanddump: Rename --nobad to --noskipbad

[Artem Bityutskiy]

On Mon, 2010-09-13 at 09:15 +0200, Wolfram Sang wrote:
> "nobad" might lead to the assumption that bad blocks are skipped, but this
> option does exactly the opposite. Use a more descriptive name.
> 
> Reported-by: Jon Povey <Jon.Povey@racelogic.co.uk>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Mike Frysinger <vapier@gentoo.org>

Pushed to mtd-2.6.git, thanks!

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