doit() is the monster function we split off from main() when we created cli_classic() and tried to introduce some abstraction. doit() is a poster child of WTFs on an astronomic scale.
Make doit() marginally less bad by factoring out self-contained code. Should have no functional changes. The new code flow is designed to make partial writes easier to implement. Tests appreciated. Signed-off-by: Carl-Daniel Hailfinger <[email protected]> Index: flashrom-doit_refactor/flashrom.c =================================================================== --- flashrom-doit_refactor/flashrom.c (Revision 1211) +++ flashrom-doit_refactor/flashrom.c (Arbeitskopie) @@ -1116,6 +1116,39 @@ return ret; } +int read_buf_from_file(unsigned char *buf, unsigned long size, char *filename) +{ + unsigned long numbytes; + FILE *image; + struct stat image_stat; + + if ((image = fopen(filename, "rb")) == NULL) { + perror(filename); + return 1; + } + if (fstat(fileno(image), &image_stat) != 0) { + perror(filename); + fclose(image); + return 1; + } + if (image_stat.st_size != size) { + msg_gerr("Error: Image size doesn't match\n"); + fclose(image); + return 1; + } + numbytes = fread(buf, 1, size, image); + if (fclose(image)) { + perror(filename); + return 1; + } + if (numbytes != size) { + msg_gerr("Error: Failed to read complete file. Got %ld bytes, " + "wanted %ld!\n", numbytes, size); + return 1; + } + return 0; +} + int write_buf_to_file(unsigned char *buf, unsigned long size, char *filename) { unsigned long numbytes; @@ -1514,8 +1547,6 @@ int doit(struct flashchip *flash, int force, char *filename, int read_it, int write_it, int erase_it, int verify_it) { uint8_t *buf; - unsigned long numbytes; - FILE *image; int ret = 0; unsigned long size; @@ -1537,7 +1568,19 @@ } } - if (erase_it) { + /* Given the existence of read locks, we want to unlock for read, + * erase and write. + */ + if (flash->unlock) + flash->unlock(flash); + + if (read_it) { + ret = read_flash_to_file(flash, filename); + programmer_shutdown(); + return ret; + } + if (erase_it || write_it) { + /* Write needs erase. */ if (flash->tested & TEST_BAD_ERASE) { msg_cerr("Erase is not working on this chip. "); if (!force) { @@ -1548,34 +1591,9 @@ msg_cerr("Continuing anyway.\n"); } } - if (flash->unlock) - flash->unlock(flash); - - if (erase_flash(flash)) { - emergency_help_message(); - programmer_shutdown(); - return 1; - } - } else if (read_it) { - if (flash->unlock) - flash->unlock(flash); - - if (read_flash_to_file(flash, filename)) { - programmer_shutdown(); - return 1; - } - } else if (write_it) { - if (flash->tested & TEST_BAD_ERASE) { - msg_cerr("Erase is not working on this chip " - "and erase is needed for write. "); - if (!force) { - msg_cerr("Aborting.\n"); - programmer_shutdown(); - return 1; - } else { - msg_cerr("Continuing anyway.\n"); - } - } + /* FIXME: Check if at least one erase function exists. */ + } + if (write_it) { if (flash->tested & TEST_BAD_WRITE) { msg_cerr("Write is not working on this chip. "); if (!force) { @@ -1591,39 +1609,25 @@ programmer_shutdown(); return 1; } - if (flash->unlock) - flash->unlock(flash); - } - if (write_it || verify_it) { - struct stat image_stat; - if ((image = fopen(filename, "rb")) == NULL) { - perror(filename); + if (erase_it) { + if (erase_flash(flash)) { + emergency_help_message(); programmer_shutdown(); - exit(1); + return 1; } - if (fstat(fileno(image), &image_stat) != 0) { - perror(filename); + } + + if (write_it || verify_it) { + if (read_buf_from_file(buf, flash->total_size * 1024, filename)) { programmer_shutdown(); - exit(1); + return 1; } - if (image_stat.st_size != flash->total_size * 1024) { - msg_gerr("Error: Image size doesn't match\n"); - programmer_shutdown(); - exit(1); - } - numbytes = fread(buf, 1, size, image); #if CONFIG_INTERNAL == 1 show_id(buf, size, force); #endif - fclose(image); - if (numbytes != size) { - msg_gerr("Error: Failed to read file. Got %ld bytes, wanted %ld!\n", numbytes, size); - programmer_shutdown(); - return 1; - } } // This should be moved into each flash part's code to do it -- http://www.hailfinger.org/ _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
