* jffs2: fix debug outputs *

PATCH In review Cyrille Pitchen 3 participants 7 messages Oct 21, 2014

Subsystems (2)

Memory technology devices (MTD) Journalling flash file system V2 (JFFS2)

lore

Share & tools

lore

Cyrille Pitchen

author

2014-10-21 08:05 UTC #0 lore raw

cover *** jffs2: fix debug outputs ***

Hi,

This series of patches fixes some bugs in the output of jffs2 debug functions.

Cyrille Pitchen (2):
jffs2: fix wrong offset in an error msg from
__jff2_dbg_prewrite_paranoia_check
jffs2: fix buffer dump debug output

 fs/jffs2/debug.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

Signature · 1 line

1.8.2.2

Cyrille Pitchen

author

2014-10-21 08:05 UTC #1 lore raw

PATCH 1/2 jffs2: fix wrong offset in an error msg from __jff2_dbg_prewrite_paranoia_check

When a none 0xff character is found in the flash buffer, an error message is printed. This error message claims to provide the offset of the FIRST corrupted byte but due to a missing break in the for loop, the former code always prints the offset of the byte immediately after the end of the tested buffer.

Signed-off-by Cyrille Pitchen <[email protected]>

---
 fs/jffs2/debug.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

fs/jffs2/debug.c +2 -3

index 1090eb6..07bd5bc 100644
@@ -141,12 +141,11 @@ __jffs2_dbg_prewrite_paranoia_check(struct jffs2_sb_info *c,
 		return;
 	}
 
-	ret = 0;
 	for (i = 0; i < len; i++)
 		if (buf[i] != 0xff)
-			ret = 1;
+			break;
 
-	if (ret) {
+	if (i != len) {
 		JFFS2_ERROR("argh, about to write node to %#08x on flash, but there are data already there. The first corrupted byte is at %#08x offset.\n",
 			ofs, ofs + i);
 		__jffs2_dbg_dump_buffer(buf, len, ofs);

Signature · 1 line

1.8.2.2

Cyrille Pitchen

author

2014-10-21 08:06 UTC #2 lore raw

PATCH 2/2 jffs2: fix buffer dump debug output

In reply to Cyrille Pitchen

This patch fixes __jffs2_dbg_dump_buffer(): this function prints a message claiming to dump len bytes. However, depending on the start offset, the former code drops up to 31 (JFFS2_BUFDUMP_BYTES_PER_LINE - 1) bytes.

Signed-off-by Cyrille Pitchen <[email protected]>

---
 fs/jffs2/debug.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

fs/jffs2/debug.c +8 ~~-13~~

index 07bd5bc..1b515b2 100644
@@ -736,30 +736,25 @@ void
 __jffs2_dbg_dump_buffer(unsigned char *buf, int len, uint32_t offs)
 {
 	int skip;
-	int i;
+	int i, pos;
 
 	printk(JFFS2_DBG_MSG_PREFIX " dump from offset %#08x to offset %#08x (%x bytes).\n",
 		offs, offs + len, len);
-	i = skip = offs % JFFS2_BUFDUMP_BYTES_PER_LINE;
+	i = skip = offs & (JFFS2_BUFDUMP_BYTES_PER_LINE - 1);
 	offs = offs & ~(JFFS2_BUFDUMP_BYTES_PER_LINE - 1);
 
-	if (skip != 0)
-		printk(JFFS2_DBG "%#08x: ", offs);
-
+	printk(JFFS2_DBG "%#08x: ", offs);
 	while (skip--)
 		printk("   ");
 
-	while (i < len) {
-		if ((i % JFFS2_BUFDUMP_BYTES_PER_LINE) == 0 && i != len -1) {
-			if (i != 0)
-				printk("\n");
+	for (pos = 0; pos < len; ++pos, ++i) {
+		if (i == JFFS2_BUFDUMP_BYTES_PER_LINE) {
 			offs += JFFS2_BUFDUMP_BYTES_PER_LINE;
-			printk(JFFS2_DBG "%0#8x: ", offs);
+			printk(JFFS2_DBG "\n%0#8x: ", offs);
+			i = 0;
 		}
 
-		printk("%02x ", buf[i]);
-
-		i += 1;
+		printk("%02x ", buf[pos]);
 	}
 
 	printk("\n");

Signature · 1 line

1.8.2.2

Joe Perches

2014-10-21 08:17 UTC #3 lore raw

Re: [PATCH 2/2] jffs2: fix buffer dump debug output

Quoted 33 lines · Cyrille Pitchen

Signed-off-by Cyrille Pitchen <[email protected]>

---
 fs/jffs2/debug.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

fs/jffs2/debug.c +3 -5

index 07bd5bc..1b515b2 100644
@@ -736,30 +736,25 @@ void
 __jffs2_dbg_dump_buffer(unsigned char *buf, int len, uint32_t offs)
 {
 	int skip;
-	int i;
+	int i, pos;
 
 	printk(JFFS2_DBG_MSG_PREFIX " dump from offset %#08x to offset %#08x (%x bytes).\n",
 		offs, offs + len, len);
-	i = skip = offs % JFFS2_BUFDUMP_BYTES_PER_LINE;
+	i = skip = offs & (JFFS2_BUFDUMP_BYTES_PER_LINE - 1);
 	offs = offs & ~(JFFS2_BUFDUMP_BYTES_PER_LINE - 1);
 
-	if (skip != 0)
-		printk(JFFS2_DBG "%#08x: ", offs);
-
+	printk(JFFS2_DBG "%#08x: ", offs);
 	while (skip--)
 		printk("   ");

fs/jffs2/debug.c void · line 754

Quoted 10 lines

fs/jffs2/debug.c +4 -5

-	while (i < len) {
-		if ((i % JFFS2_BUFDUMP_BYTES_PER_LINE) == 0 && i != len -1) {
-			if (i != 0)
-				printk("\n");
+	for (pos = 0; pos < len; ++pos, ++i) {
+		if (i == JFFS2_BUFDUMP_BYTES_PER_LINE) {
 			offs += JFFS2_BUFDUMP_BYTES_PER_LINE;
-			printk(JFFS2_DBG "%0#8x: ", offs);
+			printk(JFFS2_DBG "\n%0#8x: ", offs);
+			i = 0;

print_hex_dump would be better

Cyrille Pitchen

author

2014-10-21 09:47 UTC #4 lore raw

Re: [PATCH 2/2] jffs2: fix buffer dump debug output

Quoted 34 lines · Joe Perches

Quoted 33 lines · Cyrille Pitchen

Signed-off-by Cyrille Pitchen <[email protected]>

---
 fs/jffs2/debug.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

fs/jffs2/debug.c +3 -5

index 07bd5bc..1b515b2 100644
@@ -736,30 +736,25 @@ void
 __jffs2_dbg_dump_buffer(unsigned char *buf, int len, uint32_t offs)
 {
 	int skip;
-	int i;
+	int i, pos;
 
 	printk(JFFS2_DBG_MSG_PREFIX " dump from offset %#08x to offset %#08x (%x bytes).\n",
 		offs, offs + len, len);
-	i = skip = offs % JFFS2_BUFDUMP_BYTES_PER_LINE;
+	i = skip = offs & (JFFS2_BUFDUMP_BYTES_PER_LINE - 1);
 	offs = offs & ~(JFFS2_BUFDUMP_BYTES_PER_LINE - 1);
 
-	if (skip != 0)
-		printk(JFFS2_DBG "%#08x: ", offs);
-
+	printk(JFFS2_DBG "%#08x: ", offs);
 	while (skip--)
 		printk("   ");

Quoted 14 lines

fs/jffs2/debug.c void · line 754

Quoted 10 lines

fs/jffs2/debug.c +4 -5

-	while (i < len) {
-		if ((i % JFFS2_BUFDUMP_BYTES_PER_LINE) == 0 && i != len -1) {
-			if (i != 0)
-				printk("\n");
+	for (pos = 0; pos < len; ++pos, ++i) {
+		if (i == JFFS2_BUFDUMP_BYTES_PER_LINE) {
 			offs += JFFS2_BUFDUMP_BYTES_PER_LINE;
-			printk(JFFS2_DBG "%0#8x: ", offs);
+			printk(JFFS2_DBG "\n%0#8x: ", offs);
+			i = 0;

print_hex_dump would be better

Hi Joe,

thanks for your comment. Indeed using print_hex_dump() would be a good idea since it would avoid each driver to implement its own version of buffer dumps. Reading the source code of print_hex_dump(), the output format would change a little bit:
the output would no longer be aligned to JFFS2_BUFDUMP_BYTES_PER_LINE boundary using leading spaces.
If it's ok to change the output format it's good for me as well.

Artem Bityutskiy

2014-10-21 09:54 UTC #5 lore raw

Re: [PATCH 2/2] jffs2: fix buffer dump debug output

Quoted 7 lines · Cyrille Pitchen

thanks for your comment. Indeed using print_hex_dump() would be a good idea since it would avoid each driver to implement its own version of buffer dumps. Reading the source code of print_hex_dump(), the output format would change a little bit:
the output would no longer be aligned to JFFS2_BUFDUMP_BYTES_PER_LINE boundary using leading spaces.
If it's ok to change the output format it's good for me as well.

Changing format is OK, this is just a debugging cruft. But it is OK only if you test the changes. If you are unable to test the changes, it is better to avoid doing non-trivial changes.

Cyrille Pitchen

author

2014-10-21 10:17 UTC #6 lore raw

Re: [PATCH 2/2] jffs2: fix buffer dump debug output

Quoted 9 lines · Artem Bityutskiy

Quoted 7 lines · Cyrille Pitchen
thanks for your comment. Indeed using print_hex_dump() would be a good idea since it would avoid each driver to implement its own version of buffer dumps. Reading the source code of print_hex_dump(), the output format would change a little bit:
the output would no longer be aligned to JFFS2_BUFDUMP_BYTES_PER_LINE boundary using leading spaces.
If it's ok to change the output format it's good for me as well.

Quoted 4 lines

Changing format is OK, this is just a debugging cruft. But it is OK only if you test the changes. If you are unable to test the changes, it is better to avoid doing non-trivial changes.

The version provided in this patch was tested. However I don't know how to cope with the offs parameter of __jffs2_dbg_dump_buffer() if I replace the current source code by a call to print_hex_dump().
If I use DUMP_PREFIX_ADDRESS as prefix_type value, the printed offsets are relative to the address of buf in RAM, not to the hardware offset in my dataflash provided by the offs parameter.
Also, if I use DUMP_PREFIX_OFFSET, the printed offsets start from 0 instead of offs value.

I think keeping the right offsets in the output dump is helpfull.