Hi Max,
On Thu, Jun 9, 2016 at 3:19 PM, Max Krummenacher max.oss.09@gmail.com wrote:
Hi Benoît,
2016-06-09 1:41 GMT+02:00 Benoît Thébaudeau benoit.thebaudeau.dev@gmail.com:
On Tue, Jun 7, 2016 at 1:46 PM, Max Krummenacher max.oss.09@gmail.com wrote:
diff --git a/cmd/nand.c b/cmd/nand.c index 583a18f..8ade5e2 100644 --- a/cmd/nand.c +++ b/cmd/nand.c
[...]
if (argc > 3 && !str2off(argv[3], &size)) {Here I prefer having that in 2 if() as the stuff tested is only loosely related.
Usually, we keep things as compact as possible, which also limits the number of indentation levels, but that's fine if you prefer otherwise. I don't think it's a strong rule.
I guess keeping it like this would also require parantheses around (argc > 3).
No: `>` has higher precedence than `&&`.
Will revert to two if's in v3
puts("Size is not a valid number\n");return 1;}
return ret == 0 ? 0 : 1;
endoff = off + size;if (endoff > mtd->size) {puts("Arguments beyond end of NAND\n");return 1;}off = round_down(off, mtd->erasesize);endoff = round_up(endoff, mtd->erasesize);size = endoff - off;printf("\nNAND torture: device %d offset 0x%llx size 0x%llx ""(block size 0x%x)\n",patman.py/checkpatch.py warn here to keep quoted strings on one line even when the line length exceeds 80 characters. Will remove the line break / string concatenation for v3.
Normally, this rule is for grep-ability. Here, it's more complicated with the '%' in-between, but it's still makes sense with a regular expression, so OK.
dev, off, size, mtd->erasesize);while (off < endoff) {ret = nand_torture(mtd, off);if (ret) {failed++;printf(" block at 0x%llx failed\n", off);} else {passed++;}off += mtd->erasesize;}printf(" Passed: %u, failed: %u\n", passed, failed);return failed != 0;}#endif
Apart from above comments I merged your proposal.
@@ -775,7 +792,8 @@ static char nand_help_text[] =
...
diff --git a/doc/README.nand b/doc/README.nand index 96ffc48..5136f31 100644 --- a/doc/README.nand +++ b/doc/README.nand @@ -307,7 +307,7 @@ Miscellaneous and testing commands: DANGEROUS!!! Factory set bad blocks will be lost. Use only to remove artificial bad blocks created with the "markbad" command.
- "torture offset"
- "torture offset [size]" Torture block to determine if it is still reliable. Enabled by the CONFIG_CMD_NAND_TORTURE configuration option. This command returns 0 if the block is still reliable, else 1.
@@ -324,6 +324,10 @@ Miscellaneous and testing commands: automate actions following a nand->write() error. This would e.g. be required in order to program or update safely firmware to NAND, especially for the UBI part of such firmware.
- Optionally a second parameter size can be given to test multiple blocks with
"Optionally,"
- one call. If size is not a multiple of the NAND's erasesize then the block
"erase size, then"
- which contains offset + size will be tested in full. If used with size this
"that", not "which". "size, this"
Agreed. Will add that to v3.
Thanks.
Best regards, Benoît