[PATCH v2 0/7] new nanddump defaults

[PATCH v2 0/7] new nanddump defaults

[Brian Norris]

I sent a few of these patches without CC'ing the mailing list. Sorry for the
resend...

This series adds, changes, and deprecates several nanddump options, helping
keep them more in line with being an "inverse to nandwrite."

Brian Norris (7):
  nanddump: add --bb=METHOD option
  nanddump: remove --skipbad, leaving --bb=skipbad
  nanddump: deprecation messages for old BB options
  nanddump: warn about new default BB handling
  feature-removal-schedule: describe nanddump changes
  nanddump: add --oob option
  nanddump: document, warn about future default --omitoob

 feature-removal-schedule.txt |   36 +++++++++++
 nanddump.c                   |  136 +++++++++++++++++++++++++++++++-----------
 2 files changed, 137 insertions(+), 35 deletions(-)

[PATCH v2 1/7] nanddump: add --bb=METHOD option

[Brian Norris]

We have too many separate bad block handling methods:
 --omitbad
 --noskipbad
 --skipbad
On top of these, we have the default option: that bad blocks are
replaced with 0xFF.

These options will be unified under --bb=METHOD. The end goal will be
something like:

----------------------------------------------------------------------------------------------
Old option     New option     Comment
----------------------------------------------------------------------------------------------
<default>      --bb=padbad    dump flash data, substituting 0xFF for any bad blocks
--noskipbad    --bb=dumpbad   dump flash data, including any bad blocks
--skipbad      --bb=skipbad   dump good data, completely skipping any bad blocks (new default)
--omitbad      --bb=omitbad   dump flash data, substituting nothing for any bad blocks (DEPRECTATED)

The BB options are all mutually exclusive, so we check that we do not
have more than one BB option explicitly enabled on the command line.

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

diff --git a/nanddump.c b/nanddump.c
index c9d7e8b..07fd050 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -47,6 +47,12 @@ static void display_help(void)
 "\n"
 "           --help               Display this help and exit\n"
 "           --version            Output version information and exit\n"
+"           --bb=METHOD          Choose a bad block handling method.\n"
+"                                METHOD can be:\n"
+"                                    padbad (default)\n"
+"                                    dumpbad\n"
+"                                    skipbad\n"
+"                                    omitbad (DEPRECATED)\n"
 "-a         --forcebinary        Force printing of binary data to tty\n"
 "-b         --omitbad            Omit bad blocks from the dump\n"
 "-c         --canonicalprint     Print canonical Hex+ASCII dump\n"
@@ -60,11 +66,13 @@ static void display_help(void)
 "-q         --quiet              Don't display progress and status messages\n"
 "-s addr    --startaddress=addr  Start address\n"
 "\n"
-"Notes on --omitbad and --skipbad:\n"
-"  With either option, we stop dumping data when we encounter a bad block\n"
-"  and resume dumping at the next good block. However, with --omitbad, we\n"
+"Notes on --bb=omitbad and --bb=skipbad:\n"
+"* `omitbad' and `skipbad' are very similar; we are deprecating `omitbad'\n"
+"  in favor of `skipbad'.\n"
+"* With either option, we stop dumping data when we encounter a bad block\n"
+"  and resume dumping at the next good block. However, with `omitbad', we\n"
 "  count the bad block as part of the total dump length, whereas with\n"
-"  --skipbad, the bad block is 'skipped,' that is, not counted toward the\n"
+"  `skipbad', the bad block is skipped, that is, not counted toward the\n"
 "  total dump length.\n",
 	PROGRAM_NAME);
 	exit(EXIT_SUCCESS);
@@ -88,21 +96,26 @@ static void display_version(void)
 
 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 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;
 static bool			quiet = false;		// suppress diagnostic output
 static bool			canonical = false;	// print nice + ascii
 static bool			forcebinary = false;	// force printing binary to tty
-static bool			skipbad = false;	// skip over bad blocks
+
+static enum {
+	padbad,   // dump flash data, substituting 0xFF for any bad blocks
+	dumpbad,  // dump flash data, including any bad blocks
+	skipbad,  // dump good data, completely skipping any bad blocks
+	omitbad   // dump flash data, substituting nothing for any bad blocks (DEPRECATED)
+} bb_method = padbad;
 
 static void process_options(int argc, char * const argv[])
 {
 	int error = 0;
+	bool bb_default = true;
 
 	for (;;) {
 		int option_index = 0;
@@ -110,12 +123,13 @@ static void process_options(int argc, char * const argv[])
 		static const struct option long_options[] = {
 			{"help", no_argument, 0, 0},
 			{"version", no_argument, 0, 0},
+			{"bb", required_argument, 0, 0},
 			{"forcebinary", no_argument, 0, 'a'},
 			{"canonicalprint", no_argument, 0, 'c'},
 			{"file", required_argument, 0, 'f'},
 			{"prettyprint", no_argument, 0, 'p'},
 			{"omitoob", no_argument, 0, 'o'},
-			{"omitbad", no_argument, 0, 'b'},
+			{"omitbad", no_argument, 0, 'b'}, //DEPRECATED
 			{"startaddress", required_argument, 0, 's'},
 			{"length", required_argument, 0, 'l'},
 			{"noecc", no_argument, 0, 'n'},
@@ -140,10 +154,30 @@ static void process_options(int argc, char * const argv[])
 					case 1:
 						display_version();
 						break;
+					case 2:
+						/* Handle --bb=METHOD */
+						if (!strcmp(optarg, "padbad"))
+							bb_method = padbad;
+						else if (!strcmp(optarg, "dumpbad"))
+							bb_method = dumpbad;
+						else if (!strcmp(optarg, "skipbad"))
+							bb_method = skipbad;
+						else if (!strcmp(optarg, "omitbad"))
+							bb_method = omitbad;
+						else
+							error++;
+						bb_default = false;
+						break;
 				}
 				break;
 			case 'b':
-				omitbad = true;
+				/* Check if bb_method was already set explicitly */
+				if (bb_default) {
+					bb_default = false;
+					bb_method = omitbad;
+				} else {
+					error++;
+				}
 				break;
 			case 's':
 				start_addr = simple_strtoll(optarg, &error);
@@ -175,10 +209,22 @@ static void process_options(int argc, char * const argv[])
 				noecc = true;
 				break;
 			case 'N':
-				noskipbad = true;
+				/* Check if bb_method was already set explicitly */
+				if (bb_default) {
+					bb_default = false;
+					bb_method = dumpbad;
+				} else {
+					error++;
+				}
 				break;
 			case 'k':
-				skipbad = true;
+				/* Check if bb_method was already set explicitly */
+				if (bb_default) {
+					bb_default = false;
+					bb_method = skipbad;
+				} else {
+					error++;
+				}
 				break;
 			case '?':
 				error++;
@@ -206,19 +252,6 @@ static void process_options(int argc, char * const argv[])
 		exit(EXIT_FAILURE);
 	}
 
-	if (noskipbad && skipbad) {
-		fprintf(stderr, "The noskipbad and skipbad options are "
-				"mutually-exclusive.\n"
-				"Choose one or the other.\n");
-		exit(EXIT_FAILURE);
-	}
-
-	if (omitbad && skipbad) {
-		fprintf(stderr, "The omitbad and skipbad options are mutually-"
-				"exclusive.\nChoose one or the other.\n");
-		exit(EXIT_FAILURE);
-	}
-
 	if ((argc - optind) != 1 || error)
 		display_help();
 
@@ -416,7 +449,7 @@ int main(int argc, char * const argv[])
 	/* Dump the flash contents */
 	for (ofs = start_addr; ofs < end_addr; ofs += bs) {
 		/* Check for bad block */
-		if (noskipbad) {
+		if (bb_method == dumpbad) {
 			badblock = 0;
 		} else if (blockstart != (ofs & (~mtd.eb_size + 1)) ||
 				firstblock) {
@@ -430,14 +463,14 @@ int main(int argc, char * const argv[])
 
 		if (badblock) {
 			/* skip bad block, increase end_addr */
-			if (skipbad) {
+			if (bb_method == skipbad) {
 				end_addr += mtd.eb_size;
 				ofs += mtd.eb_size - bs;
 				if (end_addr > mtd.size)
 					end_addr = mtd.size;
 				continue;
 			}
-			if (omitbad)
+			if (bb_method == omitbad)
 				continue;
 			memset(readbuf, 0xff, bs);
 		} else {
-- 
1.7.0.4

[PATCH v2 2/7] nanddump: remove --skipbad, leaving --bb=skipbad

[Brian Norris]

We are moving to unify bad block handling methods under the --bb=METHOD
option. Since --skipbad has not been included in a mtd-utils release yet,
we can safely replace it without a deprecation and phase-out period.

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

diff --git a/nanddump.c b/nanddump.c
index 07fd050..28862a9 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -57,7 +57,6 @@ static void display_help(void)
 "-b         --omitbad            Omit bad blocks from the dump\n"
 "-c         --canonicalprint     Print canonical Hex+ASCII dump\n"
 "-f file    --file=file          Dump to file\n"
-"-k         --skipbad            Skip over bad blocks (see below)\n"
 "-l length  --length=length      Length\n"
 "-n         --noecc              Read without error correction\n"
 "-N         --noskipbad          Read without bad block skipping\n"
@@ -119,7 +118,7 @@ static void process_options(int argc, char * const argv[])
 
 	for (;;) {
 		int option_index = 0;
-		static const char *short_options = "bs:f:l:opqnNcak";
+		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},
@@ -134,7 +133,6 @@ static void process_options(int argc, char * const argv[])
 			{"length", required_argument, 0, 'l'},
 			{"noecc", no_argument, 0, 'n'},
 			{"noskipbad", no_argument, 0, 'N'},
-			{"skipbad", no_argument, 0, 'k'},
 			{"quiet", no_argument, 0, 'q'},
 			{0, 0, 0, 0},
 		};
@@ -217,15 +215,6 @@ static void process_options(int argc, char * const argv[])
 					error++;
 				}
 				break;
-			case 'k':
-				/* Check if bb_method was already set explicitly */
-				if (bb_default) {
-					bb_default = false;
-					bb_method = skipbad;
-				} else {
-					error++;
-				}
-				break;
 			case '?':
 				error++;
 				break;
-- 
1.7.0.4

[PATCH v2 3/7] nanddump: deprecation messages for old BB options

[Brian Norris]

--omitbad and --noskipbad will soon be replaced by --bb=omitbad and
--bb=dumpbad. Please plan to migrate to the new usage shortly.

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

diff --git a/nanddump.c b/nanddump.c
index 28862a9..d9f24a1 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -54,17 +54,21 @@ static void display_help(void)
 "                                    skipbad\n"
 "                                    omitbad (DEPRECATED)\n"
 "-a         --forcebinary        Force printing of binary data to tty\n"
-"-b         --omitbad            Omit bad blocks from the dump\n"
 "-c         --canonicalprint     Print canonical Hex+ASCII dump\n"
 "-f file    --file=file          Dump to file\n"
 "-l length  --length=length      Length\n"
 "-n         --noecc              Read without error correction\n"
-"-N         --noskipbad          Read without bad block skipping\n"
 "-o         --omitoob            Omit oob data\n"
 "-p         --prettyprint        Print nice (hexdump)\n"
 "-q         --quiet              Don't display progress and status messages\n"
 "-s addr    --startaddress=addr  Start address\n"
 "\n"
+"Deprecated options:\n"
+"The following options are being replaced by --bb=METHOD flags.\n"
+"Do not continue to use these options.\n"
+"-b         --omitbad            Omit bad blocks from the dump\n"
+"-N         --noskipbad          Read without bad block skipping\n"
+"\n"
 "Notes on --bb=omitbad and --bb=skipbad:\n"
 "* `omitbad' and `skipbad' are very similar; we are deprecating `omitbad'\n"
 "  in favor of `skipbad'.\n"
@@ -173,6 +177,9 @@ static void process_options(int argc, char * const argv[])
 				if (bb_default) {
 					bb_default = false;
 					bb_method = omitbad;
+					fprintf(stderr, "Warning: --omitbad is being deprecated in favor of --bb=omitbad.\n"
+							"         --omitbad will not be available in future releases.\n"
+							"         Please update your usage accordingly.\n");
 				} else {
 					error++;
 				}
@@ -211,6 +218,9 @@ static void process_options(int argc, char * const argv[])
 				if (bb_default) {
 					bb_default = false;
 					bb_method = dumpbad;
+					fprintf(stderr, "Warning: --noskipbad is being deprecated in favor of --bb=dumpbad.\n"
+							"         --noskipbad will not be available in future releases.\n"
+							"         Please update your usage accordingly.\n");
 				} else {
 					error++;
 				}
-- 
1.7.0.4

Re: [PATCH v2 3/7] nanddump: deprecation messages for old BB options

[Mike Frysinger]

On Tue, Jun 21, 2011 at 11:46, Brian Norris wrote:
> +                                       fprintf(stderr, "Warning: --omitbad is being deprecated in favor of --bb=omitbad.\n"
> +                                                       "         --omitbad will not be available in future releases.\n"
> +                                                       "         Please update your usage accordingly.\n");
> +                                       fprintf(stderr, "Warning: --noskipbad is being deprecated in favor of --bb=dumpbad.\n"
> +                                                       "         --noskipbad will not be available in future releases.\n"
> +                                                       "         Please update your usage accordingly.\n");

use warning() from common.h
-mike

[PATCH v2 4/7] nanddump: warn about new default BB handling

[Brian Norris]

In an upcoming release, we will no longer default to --bb=padbad. Instead,
the default will be --bb=skipbad. This makes nanddump a better inverse
operation to nandwrite.

This patch prints warnings for users so that they get a chance to update
their scripts before the default changes officially.

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

diff --git a/nanddump.c b/nanddump.c
index d9f24a1..81ada28 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -251,6 +251,11 @@ static void process_options(int argc, char * const argv[])
 		exit(EXIT_FAILURE);
 	}
 
+	if (bb_default)
+		fprintf(stderr, "Warning: you did not specify a default bad-block handling\n"
+				"  method. In future versions, the default will change to\n"
+				"  --bb=skipbad. Use \"nanddump --help\" for more information.\n");
+
 	if ((argc - optind) != 1 || error)
 		display_help();
 
-- 
1.7.0.4

Re: [PATCH v2 4/7] nanddump: warn about new default BB handling

[Mike Frysinger]

On Tue, Jun 21, 2011 at 11:46, Brian Norris wrote:
> +       if (bb_default)
> +               fprintf(stderr, "Warning: you did not specify a default bad-block handling\n"
> +                               "  method. In future versions, the default will change to\n"
> +                               "  --bb=skipbad. Use \"nanddump --help\" for more information.\n");

nanddump has mostly been converted to common.h, so this should use
warning() instead.
-mike

[PATCH v2 5/7] feature-removal-schedule: describe nanddump changes

[Brian Norris]

Include a few notes about the edited options and new default options in
nanddump.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 feature-removal-schedule.txt |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/feature-removal-schedule.txt b/feature-removal-schedule.txt
index a5980f7..b1bc3e4 100644
--- a/feature-removal-schedule.txt
+++ b/feature-removal-schedule.txt
@@ -40,3 +40,28 @@ This is duplicating behavior with the mtdinfo utility.  Now the util
 warns when people use it, but it'll be removed in release 1.4.6.
 
 ---------------------------
+
+4. Rename --noskipbad option, kill --omitbad, change default BB method for nanddump
+
+Our bad block handling methods are getting disorganized. They will be replaced
+by a series of --bb=METHOD options (see `nanddump --help').
+
+In addition, the previous default bad block behavior was to dump 0xFF in place
+of bad blocks. This was not a good "inverse operation" of nandwrite. Thus, the
+current default behavior has become the (new) option --bb=padbad, and in 1.4.6,
+the default will be --bb=skipbad.
+
+All of these changes will occur in 1.4.6. For now, there are appropriate
+deprecation warning messages and an alert telling about the soon-to-be-default
+options.
+
+Transition summary table:
+-----------------------------------------------------------------------------------------------------------
+ Old option     New option                 Comment
+-----------------------------------------------------------------------------------------------------------
+ <default>      --bb=padbad                dump flash data, substituting 0xFF for any bad blocks
+ --noskipbad    --bb=dumpbad               dump flash data, including any bad blocks
+ --skipbad      --bb=skipbad, <default>    dump good data, completely skipping any bad blocks (new default)
+ --omitbad      N/A                        very similar to `skipbad', will be removed soon
+
+---------------------------
-- 
1.7.0.4

[PATCH v2 6/7] nanddump: add --oob option

[Brian Norris]

Adds an explicit option for including OOB data in our data dump.
Currently, this is the default behavior, but in the next release, the
default will be to exclude OOB data. This is done to mirror the '-o'
option in nandwrite.

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

diff --git a/nanddump.c b/nanddump.c
index 81ada28..7941aa2 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -59,6 +59,7 @@ static void display_help(void)
 "-l length  --length=length      Length\n"
 "-n         --noecc              Read without error correction\n"
 "-o         --omitoob            Omit oob data\n"
+"           --oob                Dump OOB data\n"
 "-p         --prettyprint        Print nice (hexdump)\n"
 "-q         --quiet              Don't display progress and status messages\n"
 "-s addr    --startaddress=addr  Start address\n"
@@ -127,6 +128,7 @@ static void process_options(int argc, char * const argv[])
 			{"help", no_argument, 0, 0},
 			{"version", no_argument, 0, 0},
 			{"bb", required_argument, 0, 0},
+			{"oob", no_argument, 0, 0},
 			{"forcebinary", no_argument, 0, 'a'},
 			{"canonicalprint", no_argument, 0, 'c'},
 			{"file", required_argument, 0, 'f'},
@@ -170,6 +172,9 @@ static void process_options(int argc, char * const argv[])
 							error++;
 						bb_default = false;
 						break;
+					case 3: /* --oob */
+						omitoob = false;
+						break;
 				}
 				break;
 			case 'b':
-- 
1.7.0.4

[PATCH v2 7/7] nanddump: document, warn about future default --omitoob

[Brian Norris]

To work as a proper inverse to nandwrite, nanddump must not dump OOB data by
default. This patch prints a warning regarding the future change. We also
suggest using the new flag, `--oob', for transitioning the next release
with new default behavior.

Note that we are changing `-o' to mean `--oob' in the next release,
similar to `nandwrite -o'.  This is a break from previous behavior.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 feature-removal-schedule.txt |   11 +++++++++++
 nanddump.c                   |   36 ++++++++++++++++++++++++++++++------
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/feature-removal-schedule.txt b/feature-removal-schedule.txt
index b1bc3e4..8ddd3c5 100644
--- a/feature-removal-schedule.txt
+++ b/feature-removal-schedule.txt
@@ -65,3 +65,14 @@ Transition summary table:
  --omitbad      N/A                        very similar to `skipbad', will be removed soon
 
 ---------------------------
+
+5. nanddump will not dump OOB by default, kill `--omitoob'
+
+In 1.4.6, nanddump will not dump OOB by default. To dump OOB, you will have to
+explicitly use the option `--oob'. For now, there is simply a warning every
+time you use nanddump without explicitly choosing `--oob' or `--omitoob'.
+
+Note that `-o' will no longer stand for `--omitoob'. To unify with nandwrite,
+it will stand for `--oob' (Dump OOB data).
+
+---------------------------
diff --git a/nanddump.c b/nanddump.c
index 7941aa2..2955db4 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -58,8 +58,8 @@ 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"
-"-o         --omitoob            Omit oob data\n"
-"           --oob                Dump OOB data\n"
+"-o         --omitoob            Omit OOB data (default in next release)\n"
+"           --oob                Dump OOB data (current default)\n"
 "-p         --prettyprint        Print nice (hexdump)\n"
 "-q         --quiet              Don't display progress and status messages\n"
 "-s addr    --startaddress=addr  Start address\n"
@@ -77,7 +77,14 @@ static void display_help(void)
 "  and resume dumping at the next good block. However, with `omitbad', we\n"
 "  count the bad block as part of the total dump length, whereas with\n"
 "  `skipbad', the bad block is skipped, that is, not counted toward the\n"
-"  total dump length.\n",
+"  total dump length.\n"
+"\n"
+"Note on --oob, --omitoob:\n"
+"  To make nanddump act more like an inverse to nandwrite, we are changing\n"
+"  the default OOB behavior. In the next release, nanddump will not dump\n"
+"  OOB data by default. Then, we will kill the `--omitbad' option. The\n"
+"  short option `-o' will then stand for `--oob', not `--omitoob'. Please\n"
+"  adjust your usage accordingly.\n",
 	PROGRAM_NAME);
 	exit(EXIT_SUCCESS);
 }
@@ -119,7 +126,7 @@ static enum {
 static void process_options(int argc, char * const argv[])
 {
 	int error = 0;
-	bool bb_default = true;
+	bool bb_default = true, oob_default = true;
 
 	for (;;) {
 		int option_index = 0;
@@ -173,7 +180,13 @@ static void process_options(int argc, char * const argv[])
 						bb_default = false;
 						break;
 					case 3: /* --oob */
-						omitoob = false;
+						if (oob_default) {
+							oob_default = false;
+							omitoob = false;
+						} else {
+							fprintf(stderr, "Error: --oob and --oomitoob are mutually exclusive\n");
+							error++;
+						}
 						break;
 				}
 				break;
@@ -202,7 +215,13 @@ static void process_options(int argc, char * const argv[])
 				length = simple_strtoll(optarg, &error);
 				break;
 			case 'o':
-				omitoob = true;
+				if (oob_default) {
+					oob_default = false;
+					omitoob = true;
+				} else {
+					fprintf(stderr, "Error: --oob and --oomitoob are mutually exclusive\n");
+					error++;
+				}
 				break;
 			case 'a':
 				forcebinary = true;
@@ -261,6 +280,11 @@ static void process_options(int argc, char * const argv[])
 				"  method. In future versions, the default will change to\n"
 				"  --bb=skipbad. Use \"nanddump --help\" for more information.\n");
 
+	if (oob_default)
+		fprintf(stderr, "Warning: in next release, nanddump will not dump OOB\n"
+				"  by default. Use `nanddump --oob' explicitly to ensure\n"
+				"  it is dumped.\n");
+
 	if ((argc - optind) != 1 || error)
 		display_help();
 
-- 
1.7.0.4

Re: [PATCH v2 7/7] nanddump: document, warn about future default --omitoob

[Mike Frysinger]

On Tue, Jun 21, 2011 at 11:46, Brian Norris wrote:
> +                                                       fprintf(stderr, "Error: --oob and --oomitoob are mutually exclusive\n");
> +                                       fprintf(stderr, "Error: --oob and --oomitoob are mutually exclusive\n");

i would just errmsg_die() in these places

> +               fprintf(stderr, "Warning: in next release, nanddump will not dump OOB\n"
> +                               "  by default. Use `nanddump --oob' explicitly to ensure\n"
> +                               "  it is dumped.\n");

warning()
-mike

Re: [PATCH v2 0/7] new nanddump defaults

[Mike Frysinger]

other than the little style nitpicks, nothing jumped out at me.  looks
like a good idea, so:
Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike

Re: [PATCH v2 0/7] new nanddump defaults

[Brian Norris]

On Tue, Jun 21, 2011 at 12:40 PM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> other than the little style nitpicks, nothing jumped out at me.  looks
> like a good idea, so:
> Acked-by: Mike Frysinger <vapier@gentoo.org>
> -mike

Cool, thanks. I'll send v3 with these fixes tomorrow if no one else
has comments.

Brian

[PATCH v3 0/7] prepare new nanddump options, defaults

[Brian Norris]

Hello,

I think this should be the last version necessary for this particular patch
series...

This series adds, changes, and deprecates several nanddump options, helping make
nanddump a closer inverse to nandwrite. This is mostly a preparation for the
next release, when several defaults might change. A short list of the planned
changes:

 * unify bad block methods under `--bb=METHOD', deprecating old ones
 * kill --omitbad in favor of --bb=skipbad
 * skip bad blocks by default (i.e., --bb=skipbad)
 * do not dump OOB by default (--omitoob)
 * add `--oob' to force dumping OOB

Please let me know when these are accepted and the 1.4.5 release is made, so I
can send a proper patchset to clean this all up as planned for the 1.4.6
release.

BTW, is each utility supposed to have its own version? (e.g., nanddump has
version 1.30?) Should these versions be kept in any kind of sync with the
mtd-utils revision tags? And does Artem take care of this stuff?

Thanks,
Brian

Brian Norris (7):
  nanddump: add --bb=METHOD option
  nanddump: remove --skipbad, leaving --bb=skipbad
  nanddump: update help message for BB method changes
  nanddump: warn about new default BB handling
  feature-removal-schedule: describe nanddump changes
  nanddump: add --oob option
  nanddump: document, warn about future default --omitoob

 feature-removal-schedule.txt |   37 ++++++++++++
 nanddump.c                   |  132 +++++++++++++++++++++++++++++++-----------
 2 files changed, 134 insertions(+), 35 deletions(-)

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Artem Bityutskiy]

On Wed, 2011-06-22 at 09:49 -0700, Brian Norris wrote:
> Hello,
> 
> I think this should be the last version necessary for this particular patch
> series...
> 
> This series adds, changes, and deprecates several nanddump options, helping make
> nanddump a closer inverse to nandwrite. This is mostly a preparation for the
> next release, when several defaults might change. A short list of the planned
> changes:
> 
>  * unify bad block methods under `--bb=METHOD', deprecating old ones
>  * kill --omitbad in favor of --bb=skipbad
>  * skip bad blocks by default (i.e., --bb=skipbad)
>  * do not dump OOB by default (--omitoob)
>  * add `--oob' to force dumping OOB
> 
> Please let me know when these are accepted and the 1.4.5 release is made, so I
> can send a proper patchset to clean this all up as planned for the 1.4.6
> release.

Pushed, thank you!

> BTW, is each utility supposed to have its own version? (e.g., nanddump has
> version 1.30?) Should these versions be kept in any kind of sync with the
> mtd-utils revision tags? And does Artem take care of this stuff?

Actually no, I do not take care of this and this is messy. Probably it
would be easier if all utils had the same version as the release...

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Artem Bityutskiy]

On Thu, 2011-06-23 at 18:02 +0300, Artem Bityutskiy wrote:
> On Wed, 2011-06-22 at 09:49 -0700, Brian Norris wrote:
> > Hello,
> > 
> > I think this should be the last version necessary for this particular patch
> > series...
> > 
> > This series adds, changes, and deprecates several nanddump options, helping make
> > nanddump a closer inverse to nandwrite. This is mostly a preparation for the
> > next release, when several defaults might change. A short list of the planned
> > changes:
> > 
> >  * unify bad block methods under `--bb=METHOD', deprecating old ones
> >  * kill --omitbad in favor of --bb=skipbad
> >  * skip bad blocks by default (i.e., --bb=skipbad)
> >  * do not dump OOB by default (--omitoob)
> >  * add `--oob' to force dumping OOB
> > 
> > Please let me know when these are accepted and the 1.4.5 release is made, so I
> > can send a proper patchset to clean this all up as planned for the 1.4.6
> > release.
> 
> Pushed, thank you!
> 
> > BTW, is each utility supposed to have its own version? (e.g., nanddump has
> > version 1.30?) Should these versions be kept in any kind of sync with the
> > mtd-utils revision tags? And does Artem take care of this stuff?
> 
> Actually no, I do not take care of this and this is messy. Probably it
> would be easier if all utils had the same version as the release...

And speaking about released, should we do a new one? If you see that
some utils should have a version increased - feel free to do so. But
even better we could make the version numbers to be the same as release
number, then this process would be more automated and version would make
some sense...

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Mike Frysinger]

On Thu, Jun 23, 2011 at 11:04, Artem Bityutskiy wrote:
> And speaking about released, should we do a new one? If you see that
> some utils should have a version increased - feel free to do so. But
> even better we could make the version numbers to be the same as release
> number, then this process would be more automated and version would make
> some sense...

the version isnt encoded anywhere.  so if you started using something
like the kernel where the VERSION was in the Makefile, we could use
that to create a header file which all the utils would include.
-mike

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Brian Norris]

On Thu, Jun 23, 2011 at 8:52 AM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> On Thu, Jun 23, 2011 at 11:04, Artem Bityutskiy wrote:
>> And speaking about released, should we do a new one?

Yes, I think so. Then I can eliminate all those warnings and finish my changes!

>> If you see that
>> some utils should have a version increased - feel free to do so.

I think the nanddump, mtdinfo, and flash_{lock, unlock} tools have had
plenty of updates to warrant a bump...but flash_{lock, unlock} doesn't
have a version number (neither do many of the utils). I'll send a
patch for bumping soon...then should the release tag come afterward?

>> But
>> even better we could make the version numbers to be the same as release
>> number, then this process would be more automated and version would make
>> some sense...
>
> the version isnt encoded anywhere.  so if you started using something
> like the kernel where the VERSION was in the Makefile, we could use
> that to create a header file which all the utils would include.
> -mike
>

I like Mike's idea on the VERSION thing. Unfortunately, few utils
display any version number at all, and of the ones that do, we have a
random "2.1.0" from flash_erase. We wouldn't want to have a decreased
version number...so do we jump to mtd-utils v2.1 and make everything
the same version? Or even v3.0, just for fun?

If we're going for 2.1 (or some other similarly large jump), maybe we
should do the following:
1) increment some individual version numbers (nanddump and mtdinfo only?)
2) make a mtd-utils release 1.4.6
3) take care of all the "feature-removal-schedule" stuff...
4) put the VERSION in the Makefile (still version 1.4.6), have it
generate a header
5) include that header in common.h
6) add a stub version function in common.h that would utilize this
basic info to supply a "--version" option to all utilities that don't
have one (at least, as far as reasonable...)
7) release another mtd-utils, jumping to v2.x (only needing to edit
Makefile for version increments!)

How does this sound?

Brian

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Mike Frysinger]

On Thu, Jun 23, 2011 at 19:00, Brian Norris wrote:
> Unfortunately, few utils display any version number at all

i've been pondering unifying common options so all tools have the same
behavior.  for example, all utils should work the same with the basic
flags:
-h --help
-V --version
-v --verbose

it's bad enough when these deviate across projects, but even within a
project is even more obnoxious.

> and of the ones that do, we have a
> random "2.1.0" from flash_erase. We wouldn't want to have a decreased
> version number...so do we jump to mtd-utils v2.1 and make everything
> the same version? Or even v3.0, just for fun?

i dont think anyone has scripts parsing these things.  i say let's
just lock them all to the mtd release and forget about it.
-mike

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Brian Norris]

On Thu, Jun 23, 2011 at 4:14 PM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> i've been pondering unifying common options so all tools have the same
> behavior.  for example, all utils should work the same with the basic
> flags:
> -h --help
> -V --version
> -v --verbose
>
> it's bad enough when these deviate across projects, but even within a
> project is even more obnoxious.

Sounds fine. You wanna take care of that? ;)

>> and of the ones that do, we have a
>> random "2.1.0" from flash_erase. We wouldn't want to have a decreased
>> version number...so do we jump to mtd-utils v2.1 and make everything
>> the same version? Or even v3.0, just for fun?
>
> i dont think anyone has scripts parsing these things.  i say let's
> just lock them all to the mtd release and forget about it.

OK. So my list's step 7 would just have us jump to 1.4.7 and stick all
utils to that version.

I got the ball rolling with a patch incrementing nanddump and mtdinfo
versions. Then a maintainer can take step 2 on my list (i.e., tag the
release), and Mike and I can hack away at feature-removal...

Brian

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Mike Frysinger]

On Thu, Jun 23, 2011 at 19:27, Brian Norris wrote:
> On Thu, Jun 23, 2011 at 4:14 PM, Mike Frysinger wrote:
>> i've been pondering unifying common options so all tools have the same
>> behavior.  for example, all utils should work the same with the basic
>> flags:
>> -h --help
>> -V --version
>> -v --verbose
>>
>> it's bad enough when these deviate across projects, but even within a
>> project is even more obnoxious.
>
> Sounds fine. You wanna take care of that? ;)

i'll take a peek as i've done this before with other projects

> OK. So my list's step 7 would just have us jump to 1.4.7 and stick all
> utils to that version.

did someone forget to tag/push v1.4.[56] ?  i dont see it in the git repo.
-mike

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Brian Norris]

On Thu, Jun 23, 2011 at 4:36 PM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> did someone forget to tag/push v1.4.[56] ?  i dont see it in the git repo.

No, that was my step 2 :) Let me know if the steps are unreasonable.

Brian

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Mike Frysinger]

On Thu, Jun 23, 2011 at 19:40, Brian Norris wrote:
> On Thu, Jun 23, 2011 at 4:36 PM, Mike Frysinger wrote:
>> did someone forget to tag/push v1.4.[56] ?  i dont see it in the git repo.
>
> No, that was my step 2 :) Let me know if the steps are unreasonable.

i misread that.  but where's 1.4.5 ? :)
-mike

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Brian Norris]

On Thu, Jun 23, 2011 at 4:45 PM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> On Thu, Jun 23, 2011 at 19:40, Brian Norris wrote:
>> On Thu, Jun 23, 2011 at 4:36 PM, Mike Frysinger wrote:
>>> did someone forget to tag/push v1.4.[56] ?  i dont see it in the git repo.
>>
>> No, that was my step 2 :) Let me know if the steps are unreasonable.
>
> i misread that.  but where's 1.4.5 ? :)

I guess I miswrote that. Step 2 should be 1.4.5 and Step 7 should be
1.4.6. Silly me.

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Artem Bityutskiy]

On Thu, 2011-06-23 at 19:14 -0400, Mike Frysinger wrote:
> > and of the ones that do, we have a
> > random "2.1.0" from flash_erase. We wouldn't want to have a
> decreased
> > version number...so do we jump to mtd-utils v2.1 and make everything
> > the same version? Or even v3.0, just for fun?
> 
> i dont think anyone has scripts parsing these things.  i say let's
> just lock them all to the mtd release and forget about it.

Let's ask CC Peter who is packaging mtd-utils just in case. Peter, will
you suffer if mtd-utils' version all suddenly become 1.4.6? Also, will
you suffer if we switch to 2.0 or 3.0 - 2 numbers instead of 3?

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

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Peter Korsgaard]

>>>>> "Artem" == Artem Bityutskiy <dedekind1@gmail.com> writes:

Hi,

 >> i dont think anyone has scripts parsing these things.  i say let's
 >> just lock them all to the mtd release and forget about it.

 Artem> Let's ask CC Peter who is packaging mtd-utils just in
 Artem> case. Peter, will you suffer if mtd-utils' version all suddenly
 Artem> become 1.4.6? Also, will you suffer if we switch to 2.0 or 3.0 -
 Artem> 2 numbers instead of 3?

No, the version number doesn't matter as long as it increases and
release tarballs are made.

Thanks.

-- 
Bye, Peter Korsgaard

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Brian Norris]

Hello,

On Thu, Jun 23, 2011 at 4:00 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Thu, Jun 23, 2011 at 8:52 AM, Mike Frysinger <vapier.adi@gmail.com> wrote:
>> On Thu, Jun 23, 2011 at 11:04, Artem Bityutskiy wrote:
>>> But
>>> even better we could make the version numbers to be the same as release
>>> number, then this process would be more automated and version would make
>>> some sense...
>>
>> the version isnt encoded anywhere.  so if you started using something
>> like the kernel where the VERSION was in the Makefile, we could use
>> that to create a header file which all the utils would include.
>> -mike
>
> I like Mike's idea on the VERSION thing.

I took a little look at doing this "kernel style," but I'm running
into problems. It seems like this would be a little more difficult
because of the less centralized nature of mtd-utils (in comparison
with the kernel). Plus, I'm not very good with the GNU make system and
don't wanna mess too deep into something that could easily break
various people's build systems...

Also, is it just me or is the mtd-utils build setup pretty ugly? Maybe
I'm just used to how simplistic, unified, and informative the kernel
build system is, whereas mtd-utils prints out all sorts of detailed
info at the expense of *clearly* showing what it's doing.

OK, end of complaint (for now) ; I have some tangential noobish
questions, if anyone would be kind enough to answer:

Are the "ubi-utils" and "tests" meant to have the option of compiling
separately from the other mtd-utils?
Are these tools ever distributed separately? If not, do they need to
be independently buildable? It looks to me like ubi-utils is dependent
on first compiling "lib", but otherwise, they can all be built
separately... Anyway, I think this kinda screws with the centralized
VERSION thing above.
Why is mtdinfo under ubi-utils? Perhaps it should be moved when it
replaces flash_info? I think this is a fair interpretation of Item 3
in the feature-removal plan...

Thanks for putting up with me.

Cheers,
Brian

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Mike Frysinger]

On Fri, Jun 24, 2011 at 13:37, Brian Norris wrote:
> I took a little look at doing this "kernel style," but I'm running
> into problems. It seems like this would be a little more difficult
> because of the less centralized nature of mtd-utils (in comparison
> with the kernel). Plus, I'm not very good with the GNU make system and
> don't wanna mess too deep into something that could easily break
> various people's build systems...

yes, it'd take a little bit of loving to make it happen

> Also, is it just me or is the mtd-utils build setup pretty ugly? Maybe
> I'm just used to how simplistic, unified, and informative the kernel
> build system is, whereas mtd-utils prints out all sorts of detailed
> info at the expense of *clearly* showing what it's doing.

you're used to the extra work that the kernel does to make the output
pretty.  if you look at the amount of make code required to accomplish
that, the mtd-utils code is actually a loooot simpler.

> Are the "ubi-utils" and "tests" meant to have the option of compiling
> separately from the other mtd-utils?
> Are these tools ever distributed separately? If not, do they need to
> be independently buildable? It looks to me like ubi-utils is dependent
> on first compiling "lib", but otherwise, they can all be built
> separately... Anyway, I think this kinda screws with the centralized
> VERSION thing above.

i wouldnt mind redoing some of the build system logic to be like the
kernel in that there is no recursion ... everything is included in the
top level make file and built there.  it'd probably simplify the pain
we've seen in the past where mtd-utils attempts to auto-build
out-of-tree when cross-compiling.

> Why is mtdinfo under ubi-utils? Perhaps it should be moved when it
> replaces flash_info? I think this is a fair interpretation of Item 3
> in the feature-removal plan...

i blame Artem
-mike

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Artem Bityutskiy]

On Fri, 2011-06-24 at 14:28 -0400, Mike Frysinger wrote:
> > Why is mtdinfo under ubi-utils? Perhaps it should be moved when it
> > replaces flash_info? I think this is a fair interpretation of Item 3
> > in the feature-removal plan...
> 
> i blame Artem

Yes, fair blame, I was too lazy to make things properly and did not want
to deal with piles of ancient code, sorry.

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

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Artem Bityutskiy]

On Fri, 2011-06-24 at 10:37 -0700, Brian Norris wrote:
> Are the "ubi-utils" and "tests" meant to have the option of compiling
> separately from the other mtd-utils?

Probably not.

> Are these tools ever distributed separately? If not, do they need to
> be independently buildable?

I do not think that we need ubi-utils to be buildable separately.

>  It looks to me like ubi-utils is dependent
> on first compiling "lib", but otherwise, they can all be built
> separately... Anyway, I think this kinda screws with the centralized
> VERSION thing above.
> Why is mtdinfo under ubi-utils? Perhaps it should be moved when it
> replaces flash_info? I think this is a fair interpretation of Item 3
> in the feature-removal plan...

Yeah, it can be moved.

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

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Artem Bityutskiy]

On Thu, 2011-06-23 at 16:00 -0700, Brian Norris wrote:
> If we're going for 2.1 (or some other similarly large jump), maybe we
> should do the following:
> 1) increment some individual version numbers (nanddump and mtdinfo only?)
> 2) make a mtd-utils release 1.4.6
> 3) take care of all the "feature-removal-schedule" stuff...
> 4) put the VERSION in the Makefile (still version 1.4.6), have it
> generate a header
> 5) include that header in common.h
> 6) add a stub version function in common.h that would utilize this
> basic info to supply a "--version" option to all utilities that don't
> have one (at least, as far as reasonable...)
> 7) release another mtd-utils, jumping to v2.x (only needing to edit
> Makefile for version increments!)

Sounds good, and I personally do not mind to call it 2.0 or 3.0 or
1.4.6, you choose.

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

Re: [PATCH v3 0/7] prepare new nanddump options, defaults

[Artem Bityutskiy]

On Thu, 2011-06-23 at 11:52 -0400, Mike Frysinger wrote:
> On Thu, Jun 23, 2011 at 11:04, Artem Bityutskiy wrote:
> > And speaking about released, should we do a new one? If you see that
> > some utils should have a version increased - feel free to do so. But
> > even better we could make the version numbers to be the same as release
> > number, then this process would be more automated and version would make
> > some sense...
> 
> the version isnt encoded anywhere.  so if you started using something
> like the kernel where the VERSION was in the Makefile, we could use
> that to create a header file which all the utils would include.

Absolutely agree, but I've changed my employer recently and now very
busy and barely have time to follow the mailing list, so I'd be happy if
someone sent a patch :-)

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

[PATCH v3 1/7] nanddump: add --bb=METHOD option

[Brian Norris]

We have too many separate bad block handling methods:
 --omitbad
 --noskipbad
 --skipbad
On top of these, we have the default option: that bad blocks are
replaced with 0xFF.

These options will be unified under --bb=METHOD. The end goal will be
something like:

----------------------------------------------------------------------------------------------
Old option     New option     Comment
----------------------------------------------------------------------------------------------
<default>      --bb=padbad    dump flash data, substituting 0xFF for any bad blocks
--noskipbad    --bb=dumpbad   dump flash data, including any bad blocks
--skipbad      --bb=skipbad   dump good data, completely skipping any bad blocks (new default)
--omitbad      N/A            very similar to `skipbad' (DEPRECTATED)

The BB options are all mutually exclusive, so we check that we do not
have more than one BB option explicitly enabled on the command line by
tracking whether or not a BB method has been set by the user.

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

diff --git a/nanddump.c b/nanddump.c
index c9d7e8b..08f8964 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -88,21 +88,26 @@ static void display_version(void)
 
 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 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;
 static bool			quiet = false;		// suppress diagnostic output
 static bool			canonical = false;	// print nice + ascii
 static bool			forcebinary = false;	// force printing binary to tty
-static bool			skipbad = false;	// skip over bad blocks
+
+static enum {
+	padbad,   // dump flash data, substituting 0xFF for any bad blocks
+	dumpbad,  // dump flash data, including any bad blocks
+	skipbad,  // dump good data, completely skipping any bad blocks
+	omitbad   // dump flash data, substituting nothing for any bad blocks (DEPRECATED)
+} bb_method = padbad;
 
 static void process_options(int argc, char * const argv[])
 {
 	int error = 0;
+	bool bb_default = true;
 
 	for (;;) {
 		int option_index = 0;
@@ -110,6 +115,7 @@ static void process_options(int argc, char * const argv[])
 		static const struct option long_options[] = {
 			{"help", no_argument, 0, 0},
 			{"version", no_argument, 0, 0},
+			{"bb", required_argument, 0, 0},
 			{"forcebinary", no_argument, 0, 'a'},
 			{"canonicalprint", no_argument, 0, 'c'},
 			{"file", required_argument, 0, 'f'},
@@ -140,10 +146,28 @@ static void process_options(int argc, char * const argv[])
 					case 1:
 						display_version();
 						break;
+					case 2:
+						/* Handle --bb=METHOD */
+						if (!strcmp(optarg, "padbad"))
+							bb_method = padbad;
+						else if (!strcmp(optarg, "dumpbad"))
+							bb_method = dumpbad;
+						else if (!strcmp(optarg, "skipbad"))
+							bb_method = skipbad;
+						else
+							error++;
+						bb_default = false;
+						break;
 				}
 				break;
 			case 'b':
-				omitbad = true;
+				/* Check if bb_method was already set explicitly */
+				if (bb_default) {
+					bb_default = false;
+					bb_method = omitbad;
+				} else {
+					error++;
+				}
 				break;
 			case 's':
 				start_addr = simple_strtoll(optarg, &error);
@@ -175,10 +199,22 @@ static void process_options(int argc, char * const argv[])
 				noecc = true;
 				break;
 			case 'N':
-				noskipbad = true;
+				/* Check if bb_method was already set explicitly */
+				if (bb_default) {
+					bb_default = false;
+					bb_method = dumpbad;
+				} else {
+					error++;
+				}
 				break;
 			case 'k':
-				skipbad = true;
+				/* Check if bb_method was already set explicitly */
+				if (bb_default) {
+					bb_default = false;
+					bb_method = skipbad;
+				} else {
+					error++;
+				}
 				break;
 			case '?':
 				error++;
@@ -206,19 +242,6 @@ static void process_options(int argc, char * const argv[])
 		exit(EXIT_FAILURE);
 	}
 
-	if (noskipbad && skipbad) {
-		fprintf(stderr, "The noskipbad and skipbad options are "
-				"mutually-exclusive.\n"
-				"Choose one or the other.\n");
-		exit(EXIT_FAILURE);
-	}
-
-	if (omitbad && skipbad) {
-		fprintf(stderr, "The omitbad and skipbad options are mutually-"
-				"exclusive.\nChoose one or the other.\n");
-		exit(EXIT_FAILURE);
-	}
-
 	if ((argc - optind) != 1 || error)
 		display_help();
 
@@ -416,7 +439,7 @@ int main(int argc, char * const argv[])
 	/* Dump the flash contents */
 	for (ofs = start_addr; ofs < end_addr; ofs += bs) {
 		/* Check for bad block */
-		if (noskipbad) {
+		if (bb_method == dumpbad) {
 			badblock = 0;
 		} else if (blockstart != (ofs & (~mtd.eb_size + 1)) ||
 				firstblock) {
@@ -430,14 +453,14 @@ int main(int argc, char * const argv[])
 
 		if (badblock) {
 			/* skip bad block, increase end_addr */
-			if (skipbad) {
+			if (bb_method == skipbad) {
 				end_addr += mtd.eb_size;
 				ofs += mtd.eb_size - bs;
 				if (end_addr > mtd.size)
 					end_addr = mtd.size;
 				continue;
 			}
-			if (omitbad)
+			if (bb_method == omitbad)
 				continue;
 			memset(readbuf, 0xff, bs);
 		} else {
-- 
1.7.0.4

[PATCH v3 2/7] nanddump: remove --skipbad, leaving --bb=skipbad

[Brian Norris]

We are moving to unify bad block handling methods under the --bb=METHOD
option. Since --skipbad has not been included in a mtd-utils release yet,
we can safely replace it without a deprecation and phase-out period.

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

diff --git a/nanddump.c b/nanddump.c
index 08f8964..d2e2e8f 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -51,7 +51,6 @@ static void display_help(void)
 "-b         --omitbad            Omit bad blocks from the dump\n"
 "-c         --canonicalprint     Print canonical Hex+ASCII dump\n"
 "-f file    --file=file          Dump to file\n"
-"-k         --skipbad            Skip over bad blocks (see below)\n"
 "-l length  --length=length      Length\n"
 "-n         --noecc              Read without error correction\n"
 "-N         --noskipbad          Read without bad block skipping\n"
@@ -111,7 +110,7 @@ static void process_options(int argc, char * const argv[])
 
 	for (;;) {
 		int option_index = 0;
-		static const char *short_options = "bs:f:l:opqnNcak";
+		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},
@@ -126,7 +125,6 @@ static void process_options(int argc, char * const argv[])
 			{"length", required_argument, 0, 'l'},
 			{"noecc", no_argument, 0, 'n'},
 			{"noskipbad", no_argument, 0, 'N'},
-			{"skipbad", no_argument, 0, 'k'},
 			{"quiet", no_argument, 0, 'q'},
 			{0, 0, 0, 0},
 		};
@@ -207,15 +205,6 @@ static void process_options(int argc, char * const argv[])
 					error++;
 				}
 				break;
-			case 'k':
-				/* Check if bb_method was already set explicitly */
-				if (bb_default) {
-					bb_default = false;
-					bb_method = skipbad;
-				} else {
-					error++;
-				}
-				break;
 			case '?':
 				error++;
 				break;
-- 
1.7.0.4

[PATCH v3 3/7] nanddump: update help message for BB method changes

[Brian Norris]

--noskipbad will soon be replaced by --bb=dumpbad, and --omitbad is being
removed entirely in favor of --bb=skipbad.

This patch adds warnings when using inappropriate configurations and adds
info to the help message. Please plan to migrate to the new usages shortly.

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

diff --git a/nanddump.c b/nanddump.c
index d2e2e8f..103ad06 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -47,23 +47,35 @@ static void display_help(void)
 "\n"
 "           --help               Display this help and exit\n"
 "           --version            Output version information and exit\n"
+"           --bb=METHOD          Choose bad block handling method (see below).\n"
 "-a         --forcebinary        Force printing of binary data to tty\n"
-"-b         --omitbad            Omit bad blocks from the dump\n"
 "-c         --canonicalprint     Print canonical Hex+ASCII dump\n"
 "-f file    --file=file          Dump to file\n"
 "-l length  --length=length      Length\n"
 "-n         --noecc              Read without error correction\n"
-"-N         --noskipbad          Read without bad block skipping\n"
 "-o         --omitoob            Omit oob data\n"
 "-p         --prettyprint        Print nice (hexdump)\n"
 "-q         --quiet              Don't display progress and status messages\n"
 "-s addr    --startaddress=addr  Start address\n"
 "\n"
-"Notes on --omitbad and --skipbad:\n"
-"  With either option, we stop dumping data when we encounter a bad block\n"
-"  and resume dumping at the next good block. However, with --omitbad, we\n"
+"--bb=METHOD, where METHOD can be `padbad', `dumpbad', or `skipbad':\n"
+"    padbad:  dump flash data, substituting 0xFF for any bad blocks (default)\n"
+"    dumpbad: dump flash data, including any bad blocks\n"
+"    skipbad: dump good data, completely skipping any bad blocks\n"
+"\n"
+"Deprecated options:\n"
+"The following options are being replaced by --bb=METHOD flags or being\n"
+"removed entirely. Do not continue to use these options.\n"
+"-b         --omitbad            Omit bad blocks from the dump (DEPRECATED)\n"
+"-N         --noskipbad          Read without bad block skipping\n"
+"\n"
+"Notes on --omitbad and --bb=skipbad:\n"
+"* `omitbad' and `skipbad' are very similar; we are deprecating `--omitbad'\n"
+"  in favor of `--bb=skipbad'.\n"
+"* With either option, we stop dumping data when we encounter a bad block\n"
+"  and resume dumping at the next good block. However, with `omitbad', we\n"
 "  count the bad block as part of the total dump length, whereas with\n"
-"  --skipbad, the bad block is 'skipped,' that is, not counted toward the\n"
+"  `skipbad', the bad block is skipped, that is, not counted toward the\n"
 "  total dump length.\n",
 	PROGRAM_NAME);
 	exit(EXIT_SUCCESS);
@@ -120,7 +132,7 @@ static void process_options(int argc, char * const argv[])
 			{"file", required_argument, 0, 'f'},
 			{"prettyprint", no_argument, 0, 'p'},
 			{"omitoob", no_argument, 0, 'o'},
-			{"omitbad", no_argument, 0, 'b'},
+			{"omitbad", no_argument, 0, 'b'}, //DEPRECATED
 			{"startaddress", required_argument, 0, 's'},
 			{"length", required_argument, 0, 'l'},
 			{"noecc", no_argument, 0, 'n'},
@@ -163,6 +175,9 @@ static void process_options(int argc, char * const argv[])
 				if (bb_default) {
 					bb_default = false;
 					bb_method = omitbad;
+					warnmsg("--omitbad is being deprecated in favor of --bb=skipbad.\n"
+						"  --omitbad will not be available in future releases.\n"
+						"  Please update your usage accordingly.");
 				} else {
 					error++;
 				}
@@ -201,6 +216,9 @@ static void process_options(int argc, char * const argv[])
 				if (bb_default) {
 					bb_default = false;
 					bb_method = dumpbad;
+					warnmsg("--noskipbad is being deprecated in favor of --bb=dumpbad.\n"
+						"  --noskipbad will not be available in future releases.\n"
+						"  Please update your usage accordingly.");
 				} else {
 					error++;
 				}
-- 
1.7.0.4

[PATCH v3 4/7] nanddump: warn about new default BB handling

[Brian Norris]

In an upcoming release, we will no longer default to --bb=padbad. Instead,
the default will be --bb=skipbad. This makes nanddump a better inverse
operation to nandwrite.

This patch prints warnings for users so that they get a chance to update
their scripts before the default changes officially.

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

diff --git a/nanddump.c b/nanddump.c
index 103ad06..b6307cf 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -249,6 +249,11 @@ static void process_options(int argc, char * const argv[])
 		exit(EXIT_FAILURE);
 	}
 
+	if (bb_default)
+		warnmsg("you did not specify a default bad-block handling\n"
+			"  method. In future versions, the default will change to\n"
+			"  --bb=skipbad. Use \"nanddump --help\" for more information.");
+
 	if ((argc - optind) != 1 || error)
 		display_help();
 
-- 
1.7.0.4

[PATCH v3 5/7] feature-removal-schedule: describe nanddump changes

[Brian Norris]

Include a few notes about the edited options and new defaults for bad
block handling in nanddump.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 feature-removal-schedule.txt |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/feature-removal-schedule.txt b/feature-removal-schedule.txt
index a5980f7..f9910f1 100644
--- a/feature-removal-schedule.txt
+++ b/feature-removal-schedule.txt
@@ -40,3 +40,29 @@ This is duplicating behavior with the mtdinfo utility.  Now the util
 warns when people use it, but it'll be removed in release 1.4.6.
 
 ---------------------------
+
+4. Rename --noskipbad option, kill --omitbad, change default BB method for nanddump
+
+Our bad block handling methods are getting disorganized. They will be replaced
+by a series of --bb=METHOD options (see `nanddump --help').
+
+In addition, the previous default bad block behavior was to dump 0xFF in place
+of bad blocks. This was not a good "inverse operation" of nandwrite. Thus, the
+current default behavior has become the (new) option --bb=padbad, and in 1.4.6,
+the default will be --bb=skipbad.
+
+The new `--bb=METHOD' options are included in this release, but all feature
+removals and changed defaults will occur in 1.4.6. For now, there are
+appropriate deprecation warning messages and an alert telling about the
+soon-to-be-default settings.
+
+Transition summary table:
+-----------------------------------------------------------------------------------------------------------
+ Old option     New option                 Comment
+-----------------------------------------------------------------------------------------------------------
+ <default>      --bb=padbad                dump flash data, substituting 0xFF for any bad blocks
+ --noskipbad    --bb=dumpbad               dump flash data, including any bad blocks
+ --skipbad      --bb=skipbad, <default>    dump good data, completely skipping any bad blocks (new default)
+ --omitbad      N/A                        very similar to `skipbad', will be removed soon
+
+---------------------------
-- 
1.7.0.4

[PATCH v3 6/7] nanddump: add --oob option

[Brian Norris]

Adds an explicit option for including OOB data in our data dump.
Currently, this is the default behavior, but in the next release, the
default will be to exclude OOB data. This is done to mirror the '-o'
option in nandwrite.

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

diff --git a/nanddump.c b/nanddump.c
index b6307cf..e19a45f 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -54,6 +54,7 @@ static void display_help(void)
 "-l length  --length=length      Length\n"
 "-n         --noecc              Read without error correction\n"
 "-o         --omitoob            Omit oob data\n"
+"           --oob                Dump OOB data\n"
 "-p         --prettyprint        Print nice (hexdump)\n"
 "-q         --quiet              Don't display progress and status messages\n"
 "-s addr    --startaddress=addr  Start address\n"
@@ -127,6 +128,7 @@ static void process_options(int argc, char * const argv[])
 			{"help", no_argument, 0, 0},
 			{"version", no_argument, 0, 0},
 			{"bb", required_argument, 0, 0},
+			{"oob", no_argument, 0, 0},
 			{"forcebinary", no_argument, 0, 'a'},
 			{"canonicalprint", no_argument, 0, 'c'},
 			{"file", required_argument, 0, 'f'},
@@ -168,6 +170,9 @@ static void process_options(int argc, char * const argv[])
 							error++;
 						bb_default = false;
 						break;
+					case 3: /* --oob */
+						omitoob = false;
+						break;
 				}
 				break;
 			case 'b':
-- 
1.7.0.4

[PATCH v3 7/7] nanddump: document, warn about future default --omitoob

[Brian Norris]

To work as a proper inverse to nandwrite, nanddump must not dump OOB data by
default. This patch prints a warning regarding the future change. We also
suggest using the new flag, `--oob', for transitioning to the next release
with new default behavior.

Note that we are changing `-o' to mean `--oob' in the next release,
similar to `nandwrite -o'.  This is a break from previous behavior.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 feature-removal-schedule.txt |   11 +++++++++++
 nanddump.c                   |   34 ++++++++++++++++++++++++++++------
 2 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/feature-removal-schedule.txt b/feature-removal-schedule.txt
index f9910f1..338bc08 100644
--- a/feature-removal-schedule.txt
+++ b/feature-removal-schedule.txt
@@ -66,3 +66,14 @@ Transition summary table:
  --omitbad      N/A                        very similar to `skipbad', will be removed soon
 
 ---------------------------
+
+5. nanddump will not dump OOB by default
+
+In 1.4.6, nanddump will not dump OOB by default. To dump OOB, you will have to
+explicitly use the option `--oob'. For now, there is simply a warning every
+time you use nanddump without explicitly choosing `--oob' or `--omitoob'.
+
+Note that `-o' will no longer stand for `--omitoob'. To unify with nandwrite,
+it will stand for `--oob' (Dump OOB data).
+
+---------------------------
diff --git a/nanddump.c b/nanddump.c
index e19a45f..54cb1af 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -53,8 +53,8 @@ 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"
-"-o         --omitoob            Omit oob data\n"
-"           --oob                Dump OOB data\n"
+"-o         --omitoob            Omit OOB data (default in next release)\n"
+"           --oob                Dump OOB data (current default)\n"
 "-p         --prettyprint        Print nice (hexdump)\n"
 "-q         --quiet              Don't display progress and status messages\n"
 "-s addr    --startaddress=addr  Start address\n"
@@ -77,7 +77,14 @@ static void display_help(void)
 "  and resume dumping at the next good block. However, with `omitbad', we\n"
 "  count the bad block as part of the total dump length, whereas with\n"
 "  `skipbad', the bad block is skipped, that is, not counted toward the\n"
-"  total dump length.\n",
+"  total dump length.\n"
+"\n"
+"Note on --oob, --omitoob:\n"
+"  To make nanddump act more like an inverse to nandwrite, we are changing\n"
+"  the default OOB behavior. In the next release, nanddump will not dump\n"
+"  OOB data by default. We will leave both the `--omitoob' and `--oob'\n"
+"  options, but to mirror nandwrite, the short option `-o' will then stand\n"
+"  for `--oob', not `--omitoob'. Please adjust your usage accordingly.\n",
 	PROGRAM_NAME);
 	exit(EXIT_SUCCESS);
 }
@@ -119,7 +126,7 @@ static enum {
 static void process_options(int argc, char * const argv[])
 {
 	int error = 0;
-	bool bb_default = true;
+	bool bb_default = true, oob_default = true;
 
 	for (;;) {
 		int option_index = 0;
@@ -171,7 +178,12 @@ static void process_options(int argc, char * const argv[])
 						bb_default = false;
 						break;
 					case 3: /* --oob */
-						omitoob = false;
+						if (oob_default) {
+							oob_default = false;
+							omitoob = false;
+						} else {
+							errmsg_die("--oob and --oomitoob are mutually exclusive");
+						}
 						break;
 				}
 				break;
@@ -200,7 +212,12 @@ static void process_options(int argc, char * const argv[])
 				length = simple_strtoll(optarg, &error);
 				break;
 			case 'o':
-				omitoob = true;
+				if (oob_default) {
+					oob_default = false;
+					omitoob = true;
+				} else {
+					errmsg_die("--oob and --oomitoob are mutually exclusive");
+				}
 				break;
 			case 'a':
 				forcebinary = true;
@@ -259,6 +276,11 @@ static void process_options(int argc, char * const argv[])
 			"  method. In future versions, the default will change to\n"
 			"  --bb=skipbad. Use \"nanddump --help\" for more information.");
 
+	if (oob_default)
+		warnmsg("in next release, nanddump will not dump OOB\n"
+			"  by default. Use `nanddump --oob' explicitly to ensure\n"
+			"  it is dumped.");
+
 	if ((argc - optind) != 1 || error)
 		display_help();
 
-- 
1.7.0.4