[dfu-util] TI Stellaris prefix support for dfu-suffix

[Tommi Keisala]

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