[PATCH 1/3] mtd-utils: nanddump/nandwrite: Style fixups

[PATCH 1/3] mtd-utils: nanddump/nandwrite: Style fixups

[Brian Norris]

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nanddump.c  |   58 ++++++++++++++----------------
 nandwrite.c |  114 +++++++++++++++++++++++++++++------------------------------
 2 files changed, 83 insertions(+), 89 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index b7341a5..00762a5 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -37,7 +37,7 @@ static struct nand_oobinfo none_oobinfo = {
 	.useecc = MTD_NANDECC_OFF,
 };
 
-static void display_help (void)
+static void display_help(void)
 {
 	printf(
 "Usage: %s [OPTIONS] MTD-device\n"
@@ -60,7 +60,7 @@ static void display_help (void)
 	exit(EXIT_SUCCESS);
 }
 
-static void display_version (void)
+static void display_version(void)
 {
 	printf("%1$s " VERSION "\n"
 			"\n"
@@ -89,7 +89,7 @@ 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[])
+static void process_options(int argc, char * const argv[])
 {
 	int error = 0;
 
@@ -185,7 +185,7 @@ static void process_options (int argc, char * const argv[])
 	}
 
 	if ((argc - optind) != 1 || error)
-		display_help ();
+		display_help();
 
 	mtddev = argv[optind];
 }
@@ -293,14 +293,14 @@ int main(int argc, char * const argv[])
 	/* Open MTD device */
 	if ((fd = open(mtddev, O_RDONLY)) == -1) {
 		perror(mtddev);
-		exit (EXIT_FAILURE);
+		exit(EXIT_FAILURE);
 	}
 
 	/* Fill in MTD device capability structure */
 	if (ioctl(fd, MEMGETINFO, &meminfo) != 0) {
 		perror("MEMGETINFO");
 		close(fd);
-		exit (EXIT_FAILURE);
+		exit(EXIT_FAILURE);
 	}
 
 	/* Make sure device page sizes are valid */
@@ -320,32 +320,31 @@ int main(int argc, char * const argv[])
 	oob.length = meminfo.oobsize;
 
 	if (noecc)  {
-		ret = ioctl(fd, MTDFILEMODE, (void *) MTD_MODE_RAW);
+		ret = ioctl(fd, MTDFILEMODE, (void *)MTD_MODE_RAW);
 		if (ret == 0) {
 			oobinfochanged = 2;
 		} else {
 			switch (errno) {
 			case ENOTTY:
-				if (ioctl (fd, MEMGETOOBSEL, &old_oobinfo) != 0) {
-					perror ("MEMGETOOBSEL");
-					close (fd);
-					exit (EXIT_FAILURE);
+				if (ioctl(fd, MEMGETOOBSEL, &old_oobinfo) != 0) {
+					perror("MEMGETOOBSEL");
+					close(fd);
+					exit(EXIT_FAILURE);
 				}
-				if (ioctl (fd, MEMSETOOBSEL, &none_oobinfo) != 0) {
-					perror ("MEMSETOOBSEL");
-					close (fd);
-					exit (EXIT_FAILURE);
+				if (ioctl(fd, MEMSETOOBSEL, &none_oobinfo) != 0) {
+					perror("MEMSETOOBSEL");
+					close(fd);
+					exit(EXIT_FAILURE);
 				}
 				oobinfochanged = 1;
 				break;
 			default:
-				perror ("MTDFILEMODE");
-				close (fd);
-				exit (EXIT_FAILURE);
+				perror("MTDFILEMODE");
+				close(fd);
+				exit(EXIT_FAILURE);
 			}
 		}
 	} else {
-
 		/* check if we can read ecc stats */
 		if (!ioctl(fd, ECCGETSTATS, &stat1)) {
 			eccstats = true;
@@ -363,8 +362,8 @@ int main(int argc, char * const argv[])
 	 * output. */
 	if (!dumpfile) {
 		ofd = STDOUT_FILENO;
-	} else if ((ofd = open(dumpfile, O_WRONLY | O_TRUNC | O_CREAT, 0644))== -1) {
-		perror (dumpfile);
+	} else if ((ofd = open(dumpfile, O_WRONLY | O_TRUNC | O_CREAT, 0644)) == -1) {
+		perror(dumpfile);
 		close(fd);
 		exit(EXIT_FAILURE);
 	}
@@ -391,12 +390,11 @@ int main(int argc, char * const argv[])
 				meminfo.erasesize, meminfo.writesize, meminfo.oobsize);
 		fprintf(stderr,
 				"Dumping data starting at 0x%08x and ending at 0x%08x...\n",
-				(unsigned int) start_addr, (unsigned int) end_addr);
+				(unsigned int)start_addr, (unsigned int)end_addr);
 	}
 	/* Dump the flash contents */
-	for (ofs = start_addr; ofs < end_addr ; ofs+=bs) {
-
-		// new eraseblock , check for bad block
+	for (ofs = start_addr; ofs < end_addr; ofs += bs) {
+		/* Check for bad block */
 		if (noskipbad) {
 			badblock = 0;
 		} else if (blockstart != (ofs & (~meminfo.erasesize + 1))) {
@@ -446,8 +444,6 @@ int main(int argc, char * const argv[])
 		} else
 			write(ofd, readbuf, bs);
 
-
-
 		if (omitoob)
 			continue;
 
@@ -475,8 +471,8 @@ int main(int argc, char * const argv[])
 
 	/* reset oobinfo */
 	if (oobinfochanged == 1) {
-		if (ioctl (fd, MEMSETOOBSEL, &old_oobinfo) != 0) {
-			perror ("MEMSETOOBSEL");
+		if (ioctl(fd, MEMSETOOBSEL, &old_oobinfo) != 0) {
+			perror("MEMSETOOBSEL");
 			close(fd);
 			close(ofd);
 			return EXIT_FAILURE;
@@ -492,8 +488,8 @@ int main(int argc, char * const argv[])
 closeall:
 	/* The new mode change is per file descriptor ! */
 	if (oobinfochanged == 1) {
-		if (ioctl (fd, MEMSETOOBSEL, &old_oobinfo) != 0)  {
-			perror ("MEMSETOOBSEL");
+		if (ioctl(fd, MEMSETOOBSEL, &old_oobinfo) != 0)  {
+			perror("MEMSETOOBSEL");
 		}
 	}
 	close(fd);
diff --git a/nandwrite.c b/nandwrite.c
index 3520c62..e0f5f44 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -66,7 +66,7 @@ static struct nand_oobinfo autoplace_oobinfo = {
 	.useecc = MTD_NANDECC_AUTOPLACE
 };
 
-static void display_help (void)
+static void display_help(void)
 {
 	printf(
 "Usage: nandwrite [OPTION] MTD_DEVICE [INPUTFILE|-]\n"
@@ -89,10 +89,10 @@ static void display_help (void)
 "      --help              Display this help and exit\n"
 "      --version           Output version information and exit\n"
 	);
-	exit (EXIT_SUCCESS);
+	exit(EXIT_SUCCESS);
 }
 
-static void display_version (void)
+static void display_version(void)
 {
 	printf("%1$s " VERSION "\n"
 			"\n"
@@ -105,7 +105,7 @@ static void display_version (void)
 			"under the terms of the GNU General Public Licence.\n"
 			"See the file `COPYING' for more information.\n",
 			PROGRAM_NAME);
-	exit (EXIT_SUCCESS);
+	exit(EXIT_SUCCESS);
 }
 
 static const char	*standard_input = "-";
@@ -122,9 +122,9 @@ static bool		forcelegacy = false;
 static bool		noecc = false;
 static bool		noskipbad = false;
 static bool		pad = false;
-static int		blockalign = 1; /*default to using 16K block size */
+static int		blockalign = 1; /* default to using 16K block size */
 
-static void process_options (int argc, char * const argv[])
+static void process_options(int argc, char * const argv[])
 {
 	int error = 0;
 
@@ -202,10 +202,10 @@ static void process_options (int argc, char * const argv[])
 				writeoob = true;
 				break;
 			case 's':
-				mtdoffset = strtol (optarg, NULL, 0);
+				mtdoffset = strtol(optarg, NULL, 0);
 				break;
 			case 'b':
-				blockalign = atoi (optarg);
+				blockalign = atoi(optarg);
 				break;
 			case '?':
 				error++;
@@ -216,7 +216,7 @@ static void process_options (int argc, char * const argv[])
 	if (mtdoffset < 0) {
 		fprintf(stderr, "Can't specify a negative device offset `%d'\n",
 				mtdoffset);
-		exit (EXIT_FAILURE);
+		exit(EXIT_FAILURE);
 	}
 
 	argc -= optind;
@@ -228,7 +228,7 @@ static void process_options (int argc, char * const argv[])
 	 */
 
 	if (argc < 1 || argc > 2 || error)
-		display_help ();
+		display_help();
 
 	mtd_device = argv[0];
 
@@ -284,20 +284,20 @@ int main(int argc, char * const argv[])
 
 	if (pad && writeoob) {
 		fprintf(stderr, "Can't pad when oob data is present.\n");
-		exit (EXIT_FAILURE);
+		exit(EXIT_FAILURE);
 	}
 
 	/* Open the device */
 	if ((fd = open(mtd_device, O_RDWR)) == -1) {
 		perror(mtd_device);
-		exit (EXIT_FAILURE);
+		exit(EXIT_FAILURE);
 	}
 
 	/* Fill in MTD device capability structure */
 	if (ioctl(fd, MEMGETINFO, &meminfo) != 0) {
 		perror("MEMGETINFO");
 		close(fd);
-		exit (EXIT_FAILURE);
+		exit(EXIT_FAILURE);
 	}
 
 	/* Set erasesize to specified number of blocks - to match jffs2
@@ -312,7 +312,7 @@ int main(int argc, char * const argv[])
 			!(meminfo.oobsize == 128 && meminfo.writesize == 4096)) {
 		fprintf(stderr, "Unknown flash (not normal NAND)\n");
 		close(fd);
-		exit (EXIT_FAILURE);
+		exit(EXIT_FAILURE);
 	}
 
 	if (mtdoffset & (meminfo.writesize - 1)) {
@@ -325,47 +325,47 @@ int main(int argc, char * const argv[])
 
 	if (autoplace) {
 		/* Read the current oob info */
-		if (ioctl (fd, MEMGETOOBSEL, &old_oobinfo) != 0) {
-			perror ("MEMGETOOBSEL");
-			close (fd);
-			exit (EXIT_FAILURE);
+		if (ioctl(fd, MEMGETOOBSEL, &old_oobinfo) != 0) {
+			perror("MEMGETOOBSEL");
+			close(fd);
+			exit(EXIT_FAILURE);
 		}
 
 		// autoplace ECC ?
 		if (old_oobinfo.useecc != MTD_NANDECC_AUTOPLACE) {
 
-			if (ioctl (fd, MEMSETOOBSEL, &autoplace_oobinfo) != 0) {
-				perror ("MEMSETOOBSEL");
-				close (fd);
-				exit (EXIT_FAILURE);
+			if (ioctl(fd, MEMSETOOBSEL, &autoplace_oobinfo) != 0) {
+				perror("MEMSETOOBSEL");
+				close(fd);
+				exit(EXIT_FAILURE);
 			}
 			oobinfochanged = 1;
 		}
 	}
 
 	if (noecc)  {
-		ret = ioctl(fd, MTDFILEMODE, (void *) MTD_MODE_RAW);
+		ret = ioctl(fd, MTDFILEMODE, (void *)MTD_MODE_RAW);
 		if (ret == 0) {
 			oobinfochanged = 2;
 		} else {
 			switch (errno) {
 			case ENOTTY:
-				if (ioctl (fd, MEMGETOOBSEL, &old_oobinfo) != 0) {
-					perror ("MEMGETOOBSEL");
-					close (fd);
-					exit (EXIT_FAILURE);
+				if (ioctl(fd, MEMGETOOBSEL, &old_oobinfo) != 0) {
+					perror("MEMGETOOBSEL");
+					close(fd);
+					exit(EXIT_FAILURE);
 				}
-				if (ioctl (fd, MEMSETOOBSEL, &none_oobinfo) != 0) {
-					perror ("MEMSETOOBSEL");
-					close (fd);
-					exit (EXIT_FAILURE);
+				if (ioctl(fd, MEMSETOOBSEL, &none_oobinfo) != 0) {
+					perror("MEMSETOOBSEL");
+					close(fd);
+					exit(EXIT_FAILURE);
 				}
 				oobinfochanged = 1;
 				break;
 			default:
-				perror ("MTDFILEMODE");
-				close (fd);
-				exit (EXIT_FAILURE);
+				perror("MTDFILEMODE");
+				close(fd);
+				exit(EXIT_FAILURE);
 			}
 		}
 	}
@@ -387,15 +387,15 @@ int main(int argc, char * const argv[])
 		}
 		if (meminfo.oobsize == 8) {
 			if (forceyaffs) {
-				fprintf (stderr, "YAFSS cannot operate on 256 Byte page size");
+				fprintf(stderr, "YAFSS cannot operate on 256 Byte page size");
 				goto restoreoob;
 			}
 			/* Adjust number of ecc bytes */
 			jffs2_oobinfo.eccbytes = 3;
 		}
 
-		if (ioctl (fd, MEMSETOOBSEL, oobsel) != 0) {
-			perror ("MEMSETOOBSEL");
+		if (ioctl(fd, MEMSETOOBSEL, oobsel) != 0) {
+			perror("MEMSETOOBSEL");
 			goto restoreoob;
 		}
 	}
@@ -432,27 +432,27 @@ int main(int argc, char * const argv[])
 	    imglen = pagelen;
 	} else {
 	    imglen = lseek(ifd, 0, SEEK_END);
-	    lseek (ifd, 0, SEEK_SET);
+	    lseek(ifd, 0, SEEK_SET);
 	}
 
 	// Check, if file is page-aligned
 	if ((!pad) && ((imglen % pagelen) != 0)) {
-		fprintf (stderr, "Input file is not page-aligned. Use the padding "
+		fprintf(stderr, "Input file is not page-aligned. Use the padding "
 				 "option.\n");
 		goto closeall;
 	}
 
 	// Check, if length fits into device
-	if ( ((imglen / pagelen) * meminfo.writesize) > (meminfo.size - mtdoffset)) {
-		fprintf (stderr, "Image %d bytes, NAND page %d bytes, OOB area %u bytes, device size %u bytes\n",
+	if (((imglen / pagelen) * meminfo.writesize) > (meminfo.size - mtdoffset)) {
+		fprintf(stderr, "Image %d bytes, NAND page %d bytes, OOB area %u bytes, device size %u bytes\n",
 				imglen, pagelen, meminfo.writesize, meminfo.size);
-		perror ("Input file does not fit into device");
+		perror("Input file does not fit into device");
 		goto closeall;
 	}
 
 	// Allocate a buffer big enough to contain all the data (OOB included) for one eraseblock
 	filebuf_max = pagelen * meminfo.erasesize / meminfo.writesize;
-	filebuf = (unsigned char*)malloc(filebuf_max);
+	filebuf = (unsigned char *)malloc(filebuf_max);
 	if (!filebuf) {
 		fprintf(stderr, "Failed to allocate memory for file buffer (%d bytes)\n",
 				pagelen * meminfo.erasesize / meminfo.writesize);
@@ -490,7 +490,7 @@ int main(int argc, char * const argv[])
 
 			baderaseblock = false;
 			if (!quiet)
-				fprintf (stdout, "Writing data to block %d at offset 0x%x\n",
+				fprintf(stdout, "Writing data to block %d at offset 0x%x\n",
 						 blockstart / meminfo.erasesize, blockstart);
 
 			/* Check all the blocks in an erase block for bad blocks */
@@ -512,8 +512,8 @@ int main(int argc, char * const argv[])
 				if (baderaseblock) {
 					mtdoffset = blockstart + meminfo.erasesize;
 				}
-				offs +=  meminfo.erasesize / blockalign ;
-			} while ( offs < blockstart + meminfo.erasesize );
+				offs +=  meminfo.erasesize / blockalign;
+			} while (offs < blockstart + meminfo.erasesize);
 
 		}
 
@@ -529,7 +529,7 @@ int main(int argc, char * const argv[])
 				if (cnt == 0) { // EOF
 					break;
 				} else if (cnt < 0) {
-					perror ("File I/O error on input");
+					perror("File I/O error on input");
 					goto closeall;
 				}
 				tinycnt += cnt;
@@ -573,7 +573,6 @@ int main(int argc, char * const argv[])
 			// Read more data for the OOB from the input if there isn't enough in the buffer
 			if ((oobreadbuf + meminfo.oobsize) > (filebuf + filebuf_len)) {
 				int readlen = meminfo.oobsize;
-
 				int alreadyread = (filebuf + filebuf_len) - oobreadbuf;
 				int tinycnt = alreadyread;
 
@@ -582,7 +581,7 @@ int main(int argc, char * const argv[])
 					if (cnt == 0) { // EOF
 						break;
 					} else if (cnt < 0) {
-						perror ("File I/O error on input");
+						perror("File I/O error on input");
 						goto closeall;
 					}
 					tinycnt += cnt;
@@ -641,7 +640,7 @@ int main(int argc, char * const argv[])
 			/* Write OOB data first, as ecc will be placed in there*/
 			oob.start = mtdoffset;
 			if (ioctl(fd, MEMWRITEOOB, &oob) != 0) {
-				perror ("ioctl(MEMWRITEOOB)");
+				perror("ioctl(MEMWRITEOOB)");
 				goto closeall;
 			}
 		}
@@ -697,10 +696,10 @@ closeall:
 
 restoreoob:
 	if (oobinfochanged == 1) {
-		if (ioctl (fd, MEMSETOOBSEL, &old_oobinfo) != 0) {
-			perror ("MEMSETOOBSEL");
-			close (fd);
-			exit (EXIT_FAILURE);
+		if (ioctl(fd, MEMSETOOBSEL, &old_oobinfo) != 0) {
+			perror("MEMSETOOBSEL");
+			close(fd);
+			exit(EXIT_FAILURE);
 		}
 	}
 
@@ -708,10 +707,9 @@ restoreoob:
 
 	if (failed
 		|| ((ifd != STDIN_FILENO) && (imglen > 0))
-		|| (writebuf < (filebuf + filebuf_len)))
-	{
-		perror ("Data was only partially written due to error\n");
-		exit (EXIT_FAILURE);
+		|| (writebuf < (filebuf + filebuf_len))) {
+		perror("Data was only partially written due to error\n");
+		exit(EXIT_FAILURE);
 	}
 
 	/* Return happy */
-- 
1.7.0.4

[PATCH 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.

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

diff --git a/nanddump.c b/nanddump.c
index 00762a5..c71c7a6 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -265,28 +265,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;
+	static unsigned char *readbuf, *oobbuf;
 
 	process_options(argc, argv);
 
@@ -303,21 +296,22 @@ 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);
+	/* Allocate buffers */
+	oobbuf = malloc(sizeof(oobbuf) * meminfo.oobsize);
+	if (!oobbuf) {
+		perror("malloc");
+		goto closeall;
 	}
-	/* Read the real oob length */
+	readbuf = malloc(sizeof(readbuf) * meminfo.writesize);
+	if (!readbuf) {
+		perror("malloc");
+		goto closeall;
+	}
+
+	/* 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 +322,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 +355,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 +467,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;
@@ -492,7 +483,15 @@ closeall:
 			perror("MEMSETOOBSEL");
 		}
 	}
-	close(fd);
-	close(ofd);
+	if (fd) {
+		close(fd);
+		if (ofd) {
+			close(ofd);
+		}
+	}
+	if (oobbuf)
+		free(oobbuf);
+	if (readbuf)
+		free(readbuf);
 	exit(EXIT_FAILURE);
 }
-- 
1.7.0.4

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

[Mike Frysinger]

On Fri, Oct 15, 2010 at 03:44, Brian Norris wrote:
> +       static unsigned char *readbuf, *oobbuf;

no need to make these static still

> +       oobbuf = malloc(sizeof(oobbuf) * meminfo.oobsize);
> +       readbuf = malloc(sizeof(readbuf) * meminfo.writesize);

include common.h and use xmalloc

> -       close(fd);
> -       close(ofd);
> +       if (fd) {
> +               close(fd);
> +               if (ofd) {
> +                       close(ofd);
> +               }
> +       }

0 is a valid fd, thus these checks on the value of the fd dont make
much sense.  i would leave the code as it was.

> +       if (oobbuf)
> +               free(oobbuf);
> +       if (readbuf)
> +               free(readbuf);

drop the "if".  free(NULL) works just fine.
-mike

[PATCH 3/3] mtd-utils: nandwrite: Large page+oob support

[Brian Norris]

Dynamic allocation of the oob buffer provides the necessary
support for removing the oob size check. Now, new unknown OOB
sizes can be handled correctly (for example, 8KB page + 448B
OOB).

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

diff --git a/nandwrite.c b/nandwrite.c
index e0f5f44..2341694 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -42,9 +42,6 @@
 #include <asm/types.h>
 #include "mtd/mtd-user.h"
 
-#define MAX_PAGE_SIZE	4096
-#define MAX_OOB_SIZE	128
-
 // oob layouts to pass into the kernel as default
 static struct nand_oobinfo none_oobinfo = {
 	.useecc = MTD_NANDECC_OFF,
@@ -276,12 +273,10 @@ int main(int argc, char * const argv[])
 	unsigned char *writebuf = NULL;
 	// points to the OOB for the current page in filebuf
 	unsigned char *oobreadbuf = NULL;
-	unsigned char oobbuf[MAX_OOB_SIZE];
+	unsigned char *oobbuf = NULL;
 
 	process_options(argc, argv);
 
-	erase_buffer(oobbuf, sizeof(oobbuf));
-
 	if (pad && writeoob) {
 		fprintf(stderr, "Can't pad when oob data is present.\n");
 		exit(EXIT_FAILURE);
@@ -304,17 +299,6 @@ int main(int argc, char * const argv[])
 	 * (virtual) block size */
 	meminfo.erasesize *= blockalign;
 
-	/* Make sure device page sizes are valid */
-	if (!(meminfo.oobsize == 16 && meminfo.writesize == 512) &&
-			!(meminfo.oobsize == 8 && meminfo.writesize == 256) &&
-			!(meminfo.oobsize == 64 && meminfo.writesize == 2048) &&
-			!(meminfo.oobsize == 64 && meminfo.writesize == 4096) &&
-			!(meminfo.oobsize == 128 && meminfo.writesize == 4096)) {
-		fprintf(stderr, "Unknown flash (not normal NAND)\n");
-		close(fd);
-		exit(EXIT_FAILURE);
-	}
-
 	if (mtdoffset & (meminfo.writesize - 1)) {
 		fprintf(stderr, "The start address is not page-aligned !\n"
 				"The pagesize of this NAND Flash is 0x%x.\n",
@@ -455,11 +439,19 @@ int main(int argc, char * const argv[])
 	filebuf = (unsigned char *)malloc(filebuf_max);
 	if (!filebuf) {
 		fprintf(stderr, "Failed to allocate memory for file buffer (%d bytes)\n",
-				pagelen * meminfo.erasesize / meminfo.writesize);
+				filebuf_max);
 		goto closeall;
 	}
 	erase_buffer(filebuf, filebuf_max);
 
+	oobbuf = (unsigned char *)malloc(meminfo.oobsize);
+	if (!oobbuf) {
+		fprintf(stderr, "Failed to allocate memory for oob buffer (%d bytes)\n",
+				meminfo.oobsize);
+		goto closeall;
+	}
+	erase_buffer(oobbuf, meminfo.oobsize);
+
 	/*
 	 * Get data from input and write to the device while there is
 	 * still input to read and we are still within the device
@@ -688,9 +680,10 @@ int main(int argc, char * const argv[])
 	failed = false;
 
 closeall:
-	if (filebuf) {
+	if (filebuf)
 		free(filebuf);
-	}
+	if (oobbuf)
+		free(oobbuf);
 
 	close(ifd);
 
-- 
1.7.0.4

Re: [PATCH 3/3] mtd-utils: nandwrite: Large page+oob support

[Mike Frysinger]

On Fri, Oct 15, 2010 at 03:44, Brian Norris wrote:
> +       oobbuf = (unsigned char *)malloc(meminfo.oobsize);

include common.h and use xmalloc.  also, the malloc cast is useless, so drop it.
> -       if (filebuf) {
> +       if (filebuf)
>                free(filebuf);
> -       }
> +       if (oobbuf)
> +               free(oobbuf);

free(NULL) works fine, so drop the "if" check.
-mike

[PATCH v2 3/3] mtd-utils: nandwrite: Large page+oob support

[Brian Norris]

Dynamic allocation of the oob buffer provides the necessary
support for removing the oob size check. Now, new unknown OOB
sizes can be handled correctly (for example, 8KB page + 448B
OOB).

Included common.h for the use of xmalloc.

Memory freeing should occur on "restoreoob" as well as on
"closeall."

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

diff --git a/nandwrite.c b/nandwrite.c
index e0f5f44..1403378 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -41,9 +41,7 @@
 
 #include <asm/types.h>
 #include "mtd/mtd-user.h"
-
-#define MAX_PAGE_SIZE	4096
-#define MAX_OOB_SIZE	128
+#include "common.h"
 
 // oob layouts to pass into the kernel as default
 static struct nand_oobinfo none_oobinfo = {
@@ -276,12 +274,10 @@ int main(int argc, char * const argv[])
 	unsigned char *writebuf = NULL;
 	// points to the OOB for the current page in filebuf
 	unsigned char *oobreadbuf = NULL;
-	unsigned char oobbuf[MAX_OOB_SIZE];
+	unsigned char *oobbuf = NULL;
 
 	process_options(argc, argv);
 
-	erase_buffer(oobbuf, sizeof(oobbuf));
-
 	if (pad && writeoob) {
 		fprintf(stderr, "Can't pad when oob data is present.\n");
 		exit(EXIT_FAILURE);
@@ -304,17 +300,6 @@ int main(int argc, char * const argv[])
 	 * (virtual) block size */
 	meminfo.erasesize *= blockalign;
 
-	/* Make sure device page sizes are valid */
-	if (!(meminfo.oobsize == 16 && meminfo.writesize == 512) &&
-			!(meminfo.oobsize == 8 && meminfo.writesize == 256) &&
-			!(meminfo.oobsize == 64 && meminfo.writesize == 2048) &&
-			!(meminfo.oobsize == 64 && meminfo.writesize == 4096) &&
-			!(meminfo.oobsize == 128 && meminfo.writesize == 4096)) {
-		fprintf(stderr, "Unknown flash (not normal NAND)\n");
-		close(fd);
-		exit(EXIT_FAILURE);
-	}
-
 	if (mtdoffset & (meminfo.writesize - 1)) {
 		fprintf(stderr, "The start address is not page-aligned !\n"
 				"The pagesize of this NAND Flash is 0x%x.\n",
@@ -452,14 +437,12 @@ int main(int argc, char * const argv[])
 
 	// Allocate a buffer big enough to contain all the data (OOB included) for one eraseblock
 	filebuf_max = pagelen * meminfo.erasesize / meminfo.writesize;
-	filebuf = (unsigned char *)malloc(filebuf_max);
-	if (!filebuf) {
-		fprintf(stderr, "Failed to allocate memory for file buffer (%d bytes)\n",
-				pagelen * meminfo.erasesize / meminfo.writesize);
-		goto closeall;
-	}
+	filebuf = xmalloc(filebuf_max);
 	erase_buffer(filebuf, filebuf_max);
 
+	oobbuf = xmalloc(meminfo.oobsize);
+	erase_buffer(oobbuf, meminfo.oobsize);
+
 	/*
 	 * Get data from input and write to the device while there is
 	 * still input to read and we are still within the device
@@ -688,13 +671,12 @@ int main(int argc, char * const argv[])
 	failed = false;
 
 closeall:
-	if (filebuf) {
-		free(filebuf);
-	}
-
 	close(ifd);
 
 restoreoob:
+	free(filebuf);
+	free(oobbuf);
+
 	if (oobinfochanged == 1) {
 		if (ioctl(fd, MEMSETOOBSEL, &old_oobinfo) != 0) {
 			perror("MEMSETOOBSEL");
-- 
1.7.0.4

Re: [PATCH 1/3] mtd-utils: nanddump/nandwrite: Style fixups

[Artem Bityutskiy]

On Fri, 2010-10-15 at 00:44 -0700, Brian Norris wrote:
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>  nanddump.c  |   58 ++++++++++++++----------------
>  nandwrite.c |  114 +++++++++++++++++++++++++++++------------------------------
>  2 files changed, 83 insertions(+), 89 deletions(-)

Pushed to mtd-utils.git, thanks!

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