On 09/16/2012 01:51 PM, Tormod Volden wrote:
About the long option name for -s: Do you agree that
"--stellaris-address" would be better? Since its argument is really an
address, and the user still need to use the --add option as well. So
the usage would be --add --stellaris-address XXXX image.file.
I can fix that up myself, no need for resending the patch.
I agree.
+ case 's':
+ lmdfu_mode = LMDFU_ADD;
+ lmdfu_flash_address = strtoul(optarg, NULL, 0);
+ if (strcmp(optarg, "default")) {
+ lmdfu_flash_address = strtoul(optarg,&end, 0);
+ if (!lmdfu_flash_address || (*end)) {
This looks a bit strange to me, you are reading out
lmdfu_flash_address twice. And does the "default" argument value,
leaving lmdfu_flash_address zero, make any sense?
Cheers,
Tormod
Good catch.
Maybe it would be good idea to force user give proper address rather
than defaulting to 0.
I can't recall the reasoning for reading lmdfu_flash_address twice and
now it seems quite silly.
Maybe I should rewrite this part to not accept "default" and still
checking if strtoul sets *end?
Something like this:
diff --git a/src/suffix.c b/src/suffix.c
index 4ebfee7..f66300e 100644
--- a/src/suffix.c
+++ b/src/suffix.c
@@ -192,14 +192,11 @@ int main(int argc, char **argv)
break;
case 's':
lmdfu_mode = LMDFU_ADD;
- lmdfu_flash_address = strtoul(optarg, NULL, 0);
- if (strcmp(optarg, "default")) {
- lmdfu_flash_address = strtoul(optarg,
&end, 0);
- if (!lmdfu_flash_address || (*end)) {
- fprintf(stderr, "Error: Invalid
lmdfu "
- "address: %s\n", optarg);
- exit(2);
- }
+ lmdfu_flash_address = strtoul(optarg, &end, 0);
+ if (*end) {
+ fprintf(stderr, "Error: Invalid lmdfu "
+ "address: %s\n", optarg);
+ exit(2);
}
break;
case 'T':
-Tommi
_______________________________________________
devel mailing list
devel@lists.openmoko.org
https://lists.openmoko.org/mailman/listinfo/devel
