| Message ID | 1276706527-4563-1-git-send-email-norris@broadcom.com |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2010-06-16 at 09:42 -0700, Brian Norris wrote: > + if (i+15 < meminfo.oobsize) { > + /* Print 16 bytes */ > + sprintf(pretty_buf, " OOB Data: %02x %02x %02x %02x %02x %02x " > + "%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n", > + oobbuf[i], oobbuf[i+1], oobbuf[i+2], > + oobbuf[i+3], oobbuf[i+4], oobbuf[i+5], > + oobbuf[i+6], oobbuf[i+7], oobbuf[i+8], > + oobbuf[i+9], oobbuf[i+10], oobbuf[i+11], > + oobbuf[i+12], oobbuf[i+13], oobbuf[i+14], > + oobbuf[i+15]); > + write(ofd, pretty_buf, 60); > + } else { > + /* Print 10 bytes */ > + sprintf(pretty_buf, " OOB Data: %02x %02x %02x %02x %02x %02x " > + "%02x %02x %02x %02x\n", > + oobbuf[i], oobbuf[i+1], oobbuf[i+2], > + oobbuf[i+3], oobbuf[i+4], oobbuf[i+5], > + oobbuf[i+6], oobbuf[i+7], oobbuf[i+8], > + oobbuf[i+9]); > + write(ofd, pretty_buf, 42); > + } Why 10 bytes, why not 12 or 14? I think it is better to just copy-paste-modify the kernel print_hex_dump() and utilize it, instead of this ugly crocodile code ...
> Why 10 bytes, why not 12 or 14? As of now, the only OOB-size that's not a multiple of 16 supported in the other bits of ugly code is 218-bytes. 218 % 16 = 10 What's the point of checking for a "supported" page-size/OOB-size earlier if we're not gonna use that info? :) > I think it is better to just copy-paste-modify the kernel > print_hex_dump() and utilize it, instead of this ugly crocodile code ... I'm looking at that, since it seems a better solution. Does anyone oppose including ASCII output in the "pretty" option? It's built in to the print_hex_dump() already, so it shouldn't be too difficult to adapt. Anyway, it seems that someone intended to print ASCII at some point (nanddump.c, line 77): static bool pretty_print = false; // print nice in ascii
Hi, On Wed, 2010-07-07 at 11:21 -0700, Brian Norris wrote: > > Why 10 bytes, why not 12 or 14? > > As of now, the only OOB-size that's not a multiple of 16 supported in > the other bits of ugly code is 218-bytes. > > 218 % 16 = 10 > > What's the point of checking for a "supported" page-size/OOB-size > earlier if we're not gonna use that info? :) No point, as well as not much point introducing magic hard-coded constant like 10 - the printing should be generic. > > > I think it is better to just copy-paste-modify the kernel > > print_hex_dump() and utilize it, instead of this ugly crocodile code ... > > I'm looking at that, since it seems a better solution. Does anyone > oppose including ASCII output in the "pretty" option? It's built in to > the print_hex_dump() already, so it shouldn't be too difficult to adapt. > Anyway, it seems that someone intended to print ASCII at some point > (nanddump.c, line 77): > > static bool pretty_print = false; // print nice in ascii Well, it is better to make this configurable or not print ASCII, IMO, because it potentially may break scripts which use nanddump.
Hello, This patch series includes a few different fixes. Patches 1, 2, 3, and 6 are fixes to the nanddump utility, while 4 and 5 are style and minor issue patches. As per the recommendation, I adapted the linux kernel dump function for nanddump (patch 2). The patches are: [PATCH 1/6] mtd-utils/nanddump.c: Increase max OOB size [PATCH 2/6] mtd-utils/nanddump.c: Robust pretty hexdump [PATCH 3/6] mtd-utils/nanddump.c: Add canonical (hex+ascii) flag [PATCH 4/6] mtd-utils/mkfs.jffs2: fixed warnings [PATCH 5/6] mtd-utils/nandtest.c: Fixed indentation [PATCH 6/6] mtd-utils/nanddump.c: Add "forcebinary" flag Feedback is welcome, especially on the feature additions of patches 2, 3 and 6. Thanks, Brian
diff --git a/nanddump.c b/nanddump.c index e44ab36..190a653 100644 --- a/nanddump.c +++ b/nanddump.c @@ -381,15 +381,28 @@ int main(int argc, char * const argv[]) } for (i = 0; i < meminfo.oobsize; i += 16) { - sprintf(pretty_buf, " OOB Data: %02x %02x %02x %02x %02x %02x " - "%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n", - oobbuf[i], oobbuf[i+1], oobbuf[i+2], - oobbuf[i+3], oobbuf[i+4], oobbuf[i+5], - oobbuf[i+6], oobbuf[i+7], oobbuf[i+8], - oobbuf[i+9], oobbuf[i+10], oobbuf[i+11], - oobbuf[i+12], oobbuf[i+13], oobbuf[i+14], - oobbuf[i+15]); - write(ofd, pretty_buf, 60); + /* Check if we have a full 16 bytes left in the OOB */ + if (i+15 < meminfo.oobsize) { + /* Print 16 bytes */ + sprintf(pretty_buf, " OOB Data: %02x %02x %02x %02x %02x %02x " + "%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n", + oobbuf[i], oobbuf[i+1], oobbuf[i+2], + oobbuf[i+3], oobbuf[i+4], oobbuf[i+5], + oobbuf[i+6], oobbuf[i+7], oobbuf[i+8], + oobbuf[i+9], oobbuf[i+10], oobbuf[i+11], + oobbuf[i+12], oobbuf[i+13], oobbuf[i+14], + oobbuf[i+15]); + write(ofd, pretty_buf, 60); + } else { + /* Print 10 bytes */ + sprintf(pretty_buf, " OOB Data: %02x %02x %02x %02x %02x %02x " + "%02x %02x %02x %02x\n", + oobbuf[i], oobbuf[i+1], oobbuf[i+2], + oobbuf[i+3], oobbuf[i+4], oobbuf[i+5], + oobbuf[i+6], oobbuf[i+7], oobbuf[i+8], + oobbuf[i+9]); + write(ofd, pretty_buf, 42); + } } } else write(ofd, oobbuf, meminfo.oobsize);
Changed OOB output formatting in nanddump. Most NAND devices have OOB regions with multiples of 16 bytes. With newer devices, a 218-byte OOB does not split nicely into lines of 16 bytes. This patch fixes the formatting, allowing a last line of 10 bytes instead of 16. Signed-off-by: Brian Norris <norris@broadcom.com> --- nanddump.c | 31 ++++++++++++++++++++++--------- 1 files changed, 22 insertions(+), 9 deletions(-)