[PATCH v2 2/3] mtd-utils: nanddump: Dynamic buffer, increase pagesize/oobsize

[PATCH v2 2/3] mtd-utils: nanddump: Dynamic buffer, increase pagesize/oobsize

[Brian Norris]

The buffers used for dumping page and OOB data needed dynamic
allocation to help eliminate burdens for updating (i.e., every
time there's a new OOB size we don't have to increase the sizes).

Also, there is no need to check flash chips for "standard sizes."
With recent changes to the printing codebase, we should be able to
handle arbitrary sizes with no problem.

More exit operations are now necessary on program failure, so
"goto closeall" is used more liberally.

Also, common.h is included for the use of xmalloc.

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

diff --git a/nanddump.c b/nanddump.c
index 00762a5..3e7692f 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -32,6 +32,7 @@
 
 #include <asm/types.h>
 #include <mtd/mtd-user.h>
+#include "common.h"
 
 static struct nand_oobinfo none_oobinfo = {
 	.useecc = MTD_NANDECC_OFF,
@@ -265,28 +266,21 @@ nil:
 }
 
 /*
- * Buffers for reading data from flash
- */
-#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
  */
 int main(int argc, char * const argv[])
 {
 	unsigned long ofs, end_addr = 0;
 	unsigned long long blockstart = 1;
-	int ret, i, fd, ofd, bs, badblock = 0;
-	struct mtd_oob_buf oob = {0, 16, oobbuf};
+	int ret, i, fd, ofd = 0, bs, badblock = 0;
+	struct mtd_oob_buf oob;
 	mtd_info_t meminfo;
 	char pretty_buf[PRETTY_BUF_LEN];
 	int oobinfochanged = 0 ;
 	struct nand_oobinfo old_oobinfo;
 	struct mtd_ecc_stats stat1, stat2;
 	bool eccstats = false;
+	unsigned char *readbuf = NULL, *oobbuf = NULL;
 
 	process_options(argc, argv);
 
@@ -303,21 +297,14 @@ int main(int argc, char * const argv[])
 		exit(EXIT_FAILURE);
 	}
 
-	/* Make sure device page sizes are valid */
-	if (!(meminfo.oobsize == 224 && meminfo.writesize == 4096) &&
-			!(meminfo.oobsize == 218 && meminfo.writesize == 4096) &&
-			!(meminfo.oobsize == 128 && meminfo.writesize == 4096) &&
-			!(meminfo.oobsize == 64 && meminfo.writesize == 4096) &&
-			!(meminfo.oobsize == 64 && meminfo.writesize == 2048) &&
-			!(meminfo.oobsize == 32 && meminfo.writesize == 1024) &&
-			!(meminfo.oobsize == 16 && meminfo.writesize == 512) &&
-			!(meminfo.oobsize == 8 && meminfo.writesize == 256)) {
-		fprintf(stderr, "Unknown flash (not normal NAND)\n");
-		close(fd);
-		exit(EXIT_FAILURE);
-	}
-	/* Read the real oob length */
+	/* Allocate buffers */
+	oobbuf = xmalloc(sizeof(oobbuf) * meminfo.oobsize);
+	readbuf = xmalloc(sizeof(readbuf) * meminfo.writesize);
+
+	/* Fill in oob info */
+	oob.start = 0;
 	oob.length = meminfo.oobsize;
+	oob.ptr = oobbuf;
 
 	if (noecc)  {
 		ret = ioctl(fd, MTDFILEMODE, (void *)MTD_MODE_RAW);
@@ -328,20 +315,17 @@ int main(int argc, char * const argv[])
 			case ENOTTY:
 				if (ioctl(fd, MEMGETOOBSEL, &old_oobinfo) != 0) {
 					perror("MEMGETOOBSEL");
-					close(fd);
-					exit(EXIT_FAILURE);
+					goto closeall;
 				}
 				if (ioctl(fd, MEMSETOOBSEL, &none_oobinfo) != 0) {
 					perror("MEMSETOOBSEL");
-					close(fd);
-					exit(EXIT_FAILURE);
+					goto closeall;
 				}
 				oobinfochanged = 1;
 				break;
 			default:
 				perror("MTDFILEMODE");
-				close(fd);
-				exit(EXIT_FAILURE);
+				goto closeall;
 			}
 		}
 	} else {
@@ -364,15 +348,13 @@ int main(int argc, char * const argv[])
 		ofd = STDOUT_FILENO;
 	} else if ((ofd = open(dumpfile, O_WRONLY | O_TRUNC | O_CREAT, 0644)) == -1) {
 		perror(dumpfile);
-		close(fd);
-		exit(EXIT_FAILURE);
+		goto closeall;
 	}
 
 	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);
+		goto closeall;
 	}
 
 	/* Initialize start/end addresses and block size */
@@ -478,9 +460,11 @@ int main(int argc, char * const argv[])
 			return EXIT_FAILURE;
 		}
 	}
-	/* Close the output file and MTD device */
+	/* Close the output file and MTD device, free memory */
 	close(fd);
 	close(ofd);
+	free(oobbuf);
+	free(readbuf);
 
 	/* Exit happy */
 	return EXIT_SUCCESS;
@@ -494,5 +478,7 @@ closeall:
 	}
 	close(fd);
 	close(ofd);
+	free(oobbuf);
+	free(readbuf);
 	exit(EXIT_FAILURE);
 }
-- 
1.7.0.4

Re: [PATCH v2 2/3] mtd-utils: nanddump: Dynamic buffer, increase pagesize/oobsize

[Artem Bityutskiy]

On Fri, 2010-10-15 at 17:35 -0700, Brian Norris wrote:
> The buffers used for dumping page and OOB data needed dynamic
> allocation to help eliminate burdens for updating (i.e., every
> time there's a new OOB size we don't have to increase the sizes).
> 
> Also, there is no need to check flash chips for "standard sizes."
> With recent changes to the printing codebase, we should be able to
> handle arbitrary sizes with no problem.
> 
> More exit operations are now necessary on program failure, so
> "goto closeall" is used more liberally.
> 
> Also, common.h is included for the use of xmalloc.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

The patches look OK for me, but I'll wait a little in case there is
going to be more review.

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

Re: [PATCH v2 2/3] mtd-utils: nanddump: Dynamic buffer, increase pagesize/oobsize

[Artem Bityutskiy]

On Fri, 2010-10-15 at 17:35 -0700, Brian Norris wrote:
> The buffers used for dumping page and OOB data needed dynamic
> allocation to help eliminate burdens for updating (i.e., every
> time there's a new OOB size we don't have to increase the sizes).
> 
> Also, there is no need to check flash chips for "standard sizes."
> With recent changes to the printing codebase, we should be able to
> handle arbitrary sizes with no problem.
> 
> More exit operations are now necessary on program failure, so
> "goto closeall" is used more liberally.
> 
> Also, common.h is included for the use of xmalloc.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---

Patches did not apply cleanly, but I've resolved conflicts an pushed
them, thank you.

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

Re: [PATCH v2 2/3] mtd-utils: nanddump: Dynamic buffer, increase pagesize/oobsize

[Brian Norris]

On 10/17/2010 1:00 AM, Artem Bityutskiy wrote:
> Patches did not apply cleanly, but I've resolved conflicts an pushed
> them, thank you.

Perhaps this is because (as far as I can tell) you didn't actually push
the first one (style fixups) like you said you did? :) I can rebase it
and resend if that's a problem.

Brian

Re: [PATCH v2 2/3] mtd-utils: nanddump: Dynamic buffer, increase pagesize/oobsize

[Artem Bityutskiy]

On Sun, 2010-10-17 at 09:19 -0700, Brian Norris wrote:
> On 10/17/2010 1:00 AM, Artem Bityutskiy wrote:
> > Patches did not apply cleanly, but I've resolved conflicts an pushed
> > them, thank you.
> 
> Perhaps this is because (as far as I can tell) you didn't actually push
> the first one (style fixups) like you said you did? :) I can rebase it
> and resend if that's a problem.

Uhhh, I'll sort it out, apologies.

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

Re: [PATCH v2 2/3] mtd-utils: nanddump: Dynamic buffer, increase pagesize/oobsize

[Artem Bityutskiy]

On Sun, 2010-10-17 at 21:03 +0300, Artem Bityutskiy wrote:
> On Sun, 2010-10-17 at 09:19 -0700, Brian Norris wrote:
> > On 10/17/2010 1:00 AM, Artem Bityutskiy wrote:
> > > Patches did not apply cleanly, but I've resolved conflicts an pushed
> > > them, thank you.
> > 
> > Perhaps this is because (as far as I can tell) you didn't actually push
> > the first one (style fixups) like you said you did? :) I can rebase it
> > and resend if that's a problem.
> 
> Uhhh, I'll sort it out, apologies.

Done. Now pushed for real.

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