Author: hailfinger Date: Tue May 15 00:54:58 2012 New Revision: 1536 URL: http://flashrom.org/trac/flashrom/changeset/1536
Log: Convert printf to msg_* where appropriate. Clean up cli_output.c to be more readable. Use enum instead of #define for message levels. Kill a few exit(0) calls. Print the command line arguments in verbose mode. Move actions (--list-supported etc.) after argument sanity checks. Reduce the number of code paths which have their own programmer_shutdown(). Signed-off-by: Carl-Daniel Hailfinger <[email protected]> Acked-by: Stefan Tauner <[email protected]> Modified: trunk/cli_classic.c trunk/cli_output.c trunk/flash.h trunk/flashrom.c Modified: trunk/cli_classic.c ============================================================================== --- trunk/cli_classic.c Mon May 14 03:51:46 2012 (r1535) +++ trunk/cli_classic.c Tue May 15 00:54:58 2012 (r1536) @@ -159,6 +159,18 @@ exit(1); } +static int check_filename(char *filename, char *type) +{ + if (!filename || (filename[0] == '\0')) { + fprintf(stderr, "Error: No %s file specified.\n", type); + return 1; + } + /* Not an error, but maybe the user intended to specify a CLI option instead of a file name. */ + if (filename[0] == '-') + fprintf(stderr, "Warning: Supplied %s file name starts with -\n", type); + return 0; +} + int main(int argc, char *argv[]) { unsigned long size; @@ -377,36 +389,51 @@ cli_classic_abort_usage(); } - /* FIXME: Print the actions flashrom will take. */ - - if (list_supported) { - print_supported(); - exit(0); + if ((read_it | write_it | verify_it) && check_filename(filename, "image")) { + cli_classic_abort_usage(); + } + if (layoutfile && check_filename(layoutfile, "layout")) { + cli_classic_abort_usage(); } #if CONFIG_PRINT_WIKI == 1 if (list_supported_wiki) { print_supported_wiki(); - exit(0); + ret = 0; + goto out; } #endif - if (layoutfile && read_romlayout(layoutfile)) - cli_classic_abort_usage(); - if (process_include_args()) - cli_classic_abort_usage(); + if (list_supported) { + print_supported(); + ret = 0; + goto out; + } + + msg_gdbg("Command line (%i args):", argc - 1); + for (i = 0; i < argc; i++) { + msg_gdbg(" %s", argv[i]); + } + msg_gdbg("\n"); + if (layoutfile && read_romlayout(layoutfile)) { + ret = 1; + goto out; + } + if (process_include_args()) { + ret = 1; + goto out; + } /* Does a chip with the requested name exist in the flashchips array? */ if (chip_to_probe) { for (flash = flashchips; flash && flash->name; flash++) if (!strcmp(flash->name, chip_to_probe)) break; if (!flash || !flash->name) { - fprintf(stderr, "Error: Unknown chip '%s' specified.\n", - chip_to_probe); - printf("Run flashrom -L to view the hardware supported " - "in this flashrom version.\n"); - exit(1); + msg_cerr("Error: Unknown chip '%s' specified.\n", chip_to_probe); + msg_gerr("Run flashrom -L to view the hardware supported in this flashrom version.\n"); + ret = 1; + goto out; } /* Clean up after the check. */ flash = NULL; @@ -419,7 +446,7 @@ myusec_calibrate_delay(); if (programmer_init(prog, pparam)) { - fprintf(stderr, "Error: Programmer initialization failed.\n"); + msg_perr("Error: Programmer initialization failed.\n"); ret = 1; goto out_shutdown; } @@ -442,25 +469,22 @@ } if (chipcount > 1) { - printf("Multiple flash chips were detected: \"%s\"", - flashes[0].name); + msg_cinfo("Multiple flash chips were detected: \"%s\"", flashes[0].name); for (i = 1; i < chipcount; i++) - printf(", \"%s\"", flashes[i].name); - printf("\nPlease specify which chip to use with the " - "-c <chipname> option.\n"); + msg_cinfo(", \"%s\"", flashes[i].name); + msg_cinfo("\nPlease specify which chip to use with the -c <chipname> option.\n"); ret = 1; goto out_shutdown; } else if (!chipcount) { - printf("No EEPROM/flash device found.\n"); + msg_cinfo("No EEPROM/flash device found.\n"); if (!force || !chip_to_probe) { - printf("Note: flashrom can never write if the flash " - "chip isn't found automatically.\n"); + msg_cinfo("Note: flashrom can never write if the flash chip isn't found " + "automatically.\n"); } if (force && read_it && chip_to_probe) { struct registered_programmer *pgm; int compatible_programmers = 0; - printf("Force read (-f -r -c) requested, pretending " - "the chip is there:\n"); + msg_cinfo("Force read (-f -r -c) requested, pretending the chip is there:\n"); /* This loop just counts compatible controllers. */ for (j = 0; j < registered_programmer_count; j++) { pgm = ®istered_programmers[j]; @@ -468,9 +492,8 @@ compatible_programmers++; } if (compatible_programmers > 1) - printf("More than one compatible controller " - "found for the requested flash chip, " - "using the first one.\n"); + msg_cinfo("More than one compatible controller found for the requested flash " + "chip, using the first one.\n"); for (j = 0; j < registered_programmer_count; j++) { pgm = ®istered_programmers[j]; startchip = probe_flash(pgm, 0, &flashes[0], 1); @@ -478,14 +501,13 @@ break; } if (startchip == -1) { - printf("Probing for flash chip '%s' failed.\n", - chip_to_probe); + msg_cinfo("Probing for flash chip '%s' failed.\n", chip_to_probe); ret = 1; goto out_shutdown; } - printf("Please note that forced reads most likely " - "contain garbage.\n"); - return read_flash_to_file(&flashes[0], filename); + msg_cinfo("Please note that forced reads most likely contain garbage.\n"); + ret = read_flash_to_file(&flashes[0], filename); + goto out_shutdown; } ret = 1; goto out_shutdown; @@ -503,22 +525,14 @@ check_chip_supported(fill_flash); size = fill_flash->total_size * 1024; - if (check_max_decode(fill_flash->pgm->buses_supported & fill_flash->bustype, size) && - (!force)) { - fprintf(stderr, "Chip is too big for this programmer " - "(-V gives details). Use --force to override.\n"); + if (check_max_decode(fill_flash->pgm->buses_supported & fill_flash->bustype, size) && (!force)) { + msg_cerr("Chip is too big for this programmer (-V gives details). Use --force to override.\n"); ret = 1; goto out_shutdown; } if (!(read_it | write_it | verify_it | erase_it)) { - printf("No operations were specified.\n"); - goto out_shutdown; - } - - if (!filename && !erase_it) { - printf("Error: No filename specified.\n"); - ret = 1; + msg_ginfo("No operations were specified.\n"); goto out_shutdown; } @@ -531,9 +545,12 @@ * Give the chip time to settle. */ programmer_delay(100000); - return doit(fill_flash, force, filename, read_it, write_it, erase_it, verify_it); + ret |= doit(fill_flash, force, filename, read_it, write_it, erase_it, verify_it); + /* Note: doit() already calls programmer_shutdown(). */ + goto out; out_shutdown: programmer_shutdown(); +out: return ret; } Modified: trunk/cli_output.c ============================================================================== --- trunk/cli_output.c Mon May 14 03:51:46 2012 (r1535) +++ trunk/cli_output.c Tue May 15 00:54:58 2012 (r1536) @@ -22,34 +22,25 @@ #include <stdarg.h> #include "flash.h" -int print(int type, const char *fmt, ...) +/* Please note that level is the verbosity, not the importance of the message. */ +int print(enum msglevel level, const char *fmt, ...) { va_list ap; - int ret; - FILE *output_type; + int ret = 0; + FILE *output_type = stdout; - switch (type) { - case MSG_ERROR: + if (level == MSG_ERROR) output_type = stderr; - break; - case MSG_BARF: - if (verbose < 3) - return 0; - case MSG_DEBUG2: - if (verbose < 2) - return 0; - case MSG_DEBUG: - if (verbose < 1) - return 0; - case MSG_INFO: - default: - output_type = stdout; - break; - } - va_start(ap, fmt); - ret = vfprintf(output_type, fmt, ap); - va_end(ap); - fflush(output_type); + if (level <= verbose) { + va_start(ap, fmt); + ret = vfprintf(output_type, fmt, ap); + va_end(ap); + /* msg_*spew usually happens inside chip accessors in possibly + * time-critical operations. Don't slow them down by flushing. + */ + if (level != MSG_SPEW) + fflush(output_type); + } return ret; } Modified: trunk/flash.h ============================================================================== --- trunk/flash.h Mon May 14 03:51:46 2012 (r1535) +++ trunk/flash.h Tue May 15 00:54:58 2012 (r1536) @@ -268,13 +268,15 @@ #define ERROR_FLASHROM_LIMIT -201 /* cli_output.c */ +enum msglevel { + MSG_ERROR = 0, + MSG_INFO = 1, + MSG_DEBUG = 2, + MSG_DEBUG2 = 3, + MSG_SPEW = 4, +}; /* Let gcc and clang check for correct printf-style format strings. */ -int print(int type, const char *fmt, ...) __attribute__((format(printf, 2, 3))); -#define MSG_ERROR 0 -#define MSG_INFO 1 -#define MSG_DEBUG 2 -#define MSG_DEBUG2 3 -#define MSG_BARF 4 +int print(enum msglevel level, const char *fmt, ...) __attribute__((format(printf, 2, 3))); #define msg_gerr(...) print(MSG_ERROR, __VA_ARGS__) /* general errors */ #define msg_perr(...) print(MSG_ERROR, __VA_ARGS__) /* programmer errors */ #define msg_cerr(...) print(MSG_ERROR, __VA_ARGS__) /* chip errors */ @@ -287,9 +289,9 @@ #define msg_gdbg2(...) print(MSG_DEBUG2, __VA_ARGS__) /* general debug2 */ #define msg_pdbg2(...) print(MSG_DEBUG2, __VA_ARGS__) /* programmer debug2 */ #define msg_cdbg2(...) print(MSG_DEBUG2, __VA_ARGS__) /* chip debug2 */ -#define msg_gspew(...) print(MSG_BARF, __VA_ARGS__) /* general debug barf */ -#define msg_pspew(...) print(MSG_BARF, __VA_ARGS__) /* programmer debug barf */ -#define msg_cspew(...) print(MSG_BARF, __VA_ARGS__) /* chip debug barf */ +#define msg_gspew(...) print(MSG_SPEW, __VA_ARGS__) /* general debug spew */ +#define msg_pspew(...) print(MSG_SPEW, __VA_ARGS__) /* programmer debug spew */ +#define msg_cspew(...) print(MSG_SPEW, __VA_ARGS__) /* chip debug spew */ /* layout.c */ int register_include_arg(char *name); Modified: trunk/flashrom.c ============================================================================== --- trunk/flashrom.c Mon May 14 03:51:46 2012 (r1535) +++ trunk/flashrom.c Tue May 15 00:54:58 2012 (r1536) @@ -40,7 +40,7 @@ const char flashrom_version[] = FLASHROM_VERSION; char *chip_to_probe = NULL; -int verbose = 0; +int verbose = MSG_INFO; static enum programmer programmer = PROGRAMMER_INVALID; @@ -1457,27 +1457,27 @@ if (firstline) firstline = 0; else - printf("\n"); + msg_ginfo("\n"); for (i = 0; i < startcol; i++) - printf(" "); + msg_ginfo(" "); remaining = cols - startcol; } else { - printf(" "); + msg_ginfo(" "); remaining--; } if (paren && (p == 0)) { - printf("("); + msg_ginfo("("); remaining--; } - printf("%s", pname); + msg_ginfo("%s", pname); remaining -= pnamelen; if (p < PROGRAMMER_INVALID - 1) { - printf(","); + msg_ginfo(","); remaining--; } else { if (paren) - printf(")"); - printf("\n"); + msg_ginfo(")"); + msg_ginfo("\n"); } } } _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
