Skip to contentSkip to conversation

fix 2 items for nand_dump function

PATCH New Masahiro Yamada 2 participants 5 messages Jul 11, 2013 – Aug 22, 2013

lore
Share & tools
lore

Masahiro Yamada

author
#0 lore raw

cover fix 2 items for nand_dump function

These series of patches fix a memory leak
and optimize a little nand_dump function.

These 2 patches must be applied in this order.

Masahiro Yamada (2):
cmd_nand: fix a memory leak in nand_dump function
cmd_nand: slight optimization of nand_dump function

 common/cmd_nand.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)
Signature · 1 line
1.8.1.2
author
#1 lore raw

PATCH 1/2 cmd_nand: fix a memory leak in nand_dump function

If datbuf = memalign(ARCH_DMA_MINALIGN, nand->writesize); succeeds and
oobbuf = memalign(ARCH_DMA_MINALIGN, nand->oobsize);
fails, nand_dump function should free databuf.

Signed-off-by Masahiro Yamada <[email protected]>
---
 common/cmd_nand.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)
common/cmd_nand.c +16 -7
index 886212a..a66f569 100644
@@ -42,6 +42,7 @@ static int nand_dump(nand_info_t *nand, ulong off, int only_oob, int repeat)
 	int i;
 	u_char *datbuf, *oobbuf, *p;
 	static loff_t last;
+	int ret = 0;
 
 	if (repeat)
 		off = last + nand->writesize;

@@ -49,11 +50,17 @@ static int nand_dump(nand_info_t *nand, ulong off, int only_oob, int repeat)
 	last = off;
 
 	datbuf = memalign(ARCH_DMA_MINALIGN, nand->writesize);
-	oobbuf = memalign(ARCH_DMA_MINALIGN, nand->oobsize);
-	if (!datbuf || !oobbuf) {
+	if (!datbuf) {
 		puts("No memory for page buffer\n");
 		return 1;
 	}
+
+	oobbuf = memalign(ARCH_DMA_MINALIGN, nand->oobsize);
+	if (!oobbuf) {
+		puts("No memory for page buffer\n");
+		ret = 1;
+		goto free_dat;
+	}
 	off &= ~(nand->writesize - 1);
 	loff_t addr = (loff_t) off;
 	struct mtd_oob_ops ops;

@@ -66,9 +73,8 @@ static int nand_dump(nand_info_t *nand, ulong off, int only_oob, int repeat)
 	i = mtd_read_oob(nand, addr, &ops);
 	if (i < 0) {
 		printf("Error (%d) reading page %08lx\n", i, off);
-		free(datbuf);
-		free(oobbuf);
-		return 1;
+		ret = 1;
+		goto free_all;
 	}
 	printf("Page %08lx dump:\n", off);
 	i = nand->writesize >> 4;

@@ -91,10 +97,13 @@ static int nand_dump(nand_info_t *nand, ulong off, int only_oob, int repeat)
 		       p[0], p[1], p[2], p[3], p[4], p[5], p[6], p[7]);
 		p += 8;
 	}
-	free(datbuf);
+
+free_all:
 	free(oobbuf);
+free_dat:
+	free(datbuf);
 
-	return 0;
+	return ret;
 }
 
 /* ------------------------------------------------------------------------- */
Signature · 1 line
1.8.1.2
#2 lore raw

PATCH 1/2 cmd_nand: fix a memory leak in nand_dump function

Quoted 10 lines · Masahiro Yamada

If datbuf = memalign(ARCH_DMA_MINALIGN, nand->writesize); succeeds and
oobbuf = memalign(ARCH_DMA_MINALIGN, nand->oobsize);
fails, nand_dump function should free databuf.

Signed-off-by Masahiro Yamada <[email protected]>

---
common/cmd_nand.c | 23 ++++++++++++++++-------

 1 file changed, 16 insertions(+), 7 deletions(-)

Applied 1/2 and 2/2 to u-boot-nand-flash

-Scott

author
#3 lore raw

PATCH 2/2 cmd_nand: slight optimization of nand_dump function

In reply to Masahiro Yamada

If a non-zero value is given to only_oob argument, printing the main area is skipped.

With a little modification, we can skip the whole
while loop.

Signed-off-by Masahiro Yamada <[email protected]>
---
 common/cmd_nand.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
common/cmd_nand.c +8 -5
index a66f569..adc1ce4 100644
@@ -77,18 +77,21 @@ static int nand_dump(nand_info_t *nand, ulong off, int only_oob, int repeat)
 		goto free_all;
 	}
 	printf("Page %08lx dump:\n", off);
-	i = nand->writesize >> 4;
-	p = datbuf;
 
-	while (i--) {
-		if (!only_oob)
+	if (!only_oob) {
+		i = nand->writesize >> 4;
+		p = datbuf;
+
+		while (i--) {
 			printf("\t%02x %02x %02x %02x %02x %02x %02x %02x"
 			       "  %02x %02x %02x %02x %02x %02x %02x %02x\n",
 			       p[0], p[1], p[2], p[3], p[4], p[5], p[6], p[7],
 			       p[8], p[9], p[10], p[11], p[12], p[13], p[14],
 			       p[15]);
-		p += 16;
+			p += 16;
+		}
 	}
+
 	puts("OOB:\n");
 	i = nand->oobsize >> 3;
 	p = oobbuf;
Signature · 1 line
1.8.1.2
#4 lore raw

PATCH 2/2 cmd_nand: slight optimization of nand_dump function

Quoted 32 lines · Masahiro Yamada

If a non-zero value is given to only_oob argument, printing the main area is skipped.

With a little modification, we can skip the whole
while loop.

Signed-off-by Masahiro Yamada <[email protected]>
---
 common/cmd_nand.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
common/cmd_nand.c +5 -4
index a66f569..adc1ce4 100644
@@ -77,18 +77,21 @@ static int nand_dump(nand_info_t *nand, ulong off, int only_oob, int repeat)
 		goto free_all;
 	}
 	printf("Page %08lx dump:\n", off);
-	i = nand->writesize >> 4;
-	p = datbuf;
 
-	while (i--) {
-		if (!only_oob)
+	if (!only_oob) {
+		i = nand->writesize >> 4;
+		p = datbuf;
+
+		while (i--) {
 			printf("\t%02x %02x %02x %02x %02x %02x %02x %02x"
 			       "  %02x %02x %02x %02x %02x %02x %02x %02x\n",
 			       p[0], p[1], p[2], p[3], p[4], p[5], p[6], p[7],

common/cmd_nand.c static int nand_dump(nand_info_t *nand, ulong off, int only_oob, int repeat) · line 97

Quoted 10 lines
common/cmd_nand.c +3 -1
 			       p[8], p[9], p[10], p[11], p[12], p[13], p[14],
 			       p[15]);
-		p += 16;
+			p += 16;
+		}
 	}
+
 	puts("OOB:\n");
 	i = nand->oobsize >> 3;
 	p = oobbuf;

I'll probably apply this since it makes the code slightly more intuitive, but as an optimization I doubt this makes any noticeable difference.

-Scott