Am 13.06.2011 00:26 schrieb Bernd Blaauw:
Op 11-6-2011 16:21, Uwe Hermann schreef:
There's a small issue with the patch, it double-prints some lines on stdout now (in the logfile they appear only once if -o is used):
$ ./flashrom flashrom v0.9.3-r1331 on Linux 2.6.38-2-amd64 (x86_64), built with libpci 3.1.7, GCC 4.5.2, little endian flashrom is free software, get the source code at http://www.flashrom.org
On DOS platform, I see this "little endian" text truncated as ", l" "ittle endian" "flashrom is free".
must be the 80x25 default screen mode. Anyway to make this pretty instead of cutting words?
Yes. I decided to kill most of the version message in normal verbosity.
flashrom v0.9.3-r1331 on Linux 2.6.38-2-amd64 (x86_64), built with libpci 3.1.7, GCC 4.5.2, little endian Calibrating delay loop... OK. ERROR: Could not get I/O privileges (Operation not permitted). You need to be root.
Also, not all whitespace seems to be the same as in the logfile, e.g. for
$ ./flashrom -L -o foo
Seems like Uwe's right about the empty spaces, experienced the same with Idwer's "latest" DOS binary.
I "fixed" this by not logging anything if you specify -L (or any other action which won't access the chip).
Just curious: does FlashROM have any operations that don't require root on Linux?
Yes. For tests, we have the dummy programmer and flash chip emulation. The man page has some info about that. flashrom -p dummy
Otherwise you'd simply give the "you need to be root" warning before doing calibration loop, saving a second of waiting time or so?
Dummy wants calibration as well, but I plan to move that into programmer init soon.
Last cosmetic gripe: "bad command or filename" printed 6 times in case FlashROM can't find the (hopefully optional?) DOS "DMIDECOD" program.
Can you start a new thread about that? We plan to disable external dmidecode support soon, but until then we should not annoy people that much.
Is the logfile purely optional? A plain simple bootCD with DOS, FlashROM and BIOS file means a read-only filesystem.
Yes, logfile will be optional.
As not all writes might be succesfull, is it an idea to read the BIOS backup in pogram memory (instead of storing on disk) and offer to restore it in case writing new file goes bad?
We already have parts of such a recovery logic, I hope to finish it for 0.9.4.
That would be nice on read-only platforms (or those without enough space to write file to)
Sure.
Regards, Carl-Daniel
New version.
Add log file support to flashrom.
The log file will always contain all verbose messages even if the user doesn't specify -V. If the user specifies -VV, SPEW messages will be logged as well.
Please note that the log file will be empty if no programmer is accessed. This affects --list-supported --list-supported-wiki --help --version.
Only print flashrom version and OS in default output, downgrade compiler/libpci/endianness info to verbose output.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-logfile/flash.h =================================================================== --- flashrom-logfile/flash.h (Revision 1337) +++ flashrom-logfile/flash.h (Arbeitskopie) @@ -229,6 +229,12 @@ #define ERROR_NONFATAL 0x100
/* cli_output.c */ +#ifndef STANDALONE +int open_logfile(const char * const filename); +int close_logfile(void); +void start_logging(void); +#endif +int msg_log(const char *fmt, ...); /* 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 Index: flashrom-logfile/cli_output.c =================================================================== --- flashrom-logfile/cli_output.c (Revision 1337) +++ flashrom-logfile/cli_output.c (Arbeitskopie) @@ -2,6 +2,7 @@ * This file is part of the flashrom project. * * Copyright (C) 2009 Sean Nelson audiohacked@gmail.com + * Copyright (C) 2011 Carl-Daniel Hailfinger * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -22,30 +23,96 @@ #include <stdarg.h> #include "flash.h"
-int print(int type, const char *fmt, ...) +static FILE *logfile = NULL; +static int want_log = 0; + +#ifndef STANDALONE +int close_logfile(void) { + if (logfile && fclose(logfile)) + return 1; + return 0; +} + +int open_logfile(const char * const filename) +{ + if (!filename) { + msg_gerr("No filename specified.\n"); + return 1; + } + if ((logfile = fopen(filename, "w")) == NULL) { + perror(filename); + return 1; + } + return 0; +} + +void start_logging(void) +{ + int oldverbose = verbose; + + want_log = 1; + /* Shut up the console. */ + verbose = -2; + print_version(); + verbose = oldverbose; +} +#endif /* STANDALONE */ + +int msg_log(const char *fmt, ...) +{ va_list ap; int ret; - FILE *output_type;
+ if (!logfile) + return -1; + va_start(ap, fmt); + ret = vfprintf(logfile, fmt, ap); + va_end(ap); + return ret; +} + +int print(int type, const char *fmt, ...) +{ + va_list ap; + int ret = 0; + int want_screen = 1; + int want_file = 1; + FILE *output_type = stdout; + switch (type) { - case MSG_ERROR: - output_type = stderr; + case MSG_BARF: + if (verbose < 2) { + want_screen = 0; + want_file = 0; + } break; - case MSG_BARF: - if (verbose < 2) - return 0; case MSG_DEBUG: if (verbose < 1) - return 0; + want_screen = 0; + break; case MSG_INFO: + if (verbose < 0) + want_screen = 0; + break; + case MSG_ERROR: + if (verbose < -1) + want_screen = 0; + output_type = stderr; + break; default: - output_type = stdout; break; }
- va_start(ap, fmt); - ret = vfprintf(output_type, fmt, ap); - va_end(ap); + if (want_screen) { + va_start(ap, fmt); + ret = vfprintf(output_type, fmt, ap); + va_end(ap); + } + if (want_file && logfile && want_log) { + va_start(ap, fmt); + ret = vfprintf(logfile, fmt, ap); + va_end(ap); + } return ret; } Index: flashrom-logfile/cli_classic.c =================================================================== --- flashrom-logfile/cli_classic.c (Revision 1337) +++ flashrom-logfile/cli_classic.c (Arbeitskopie) @@ -38,7 +38,8 @@ "-z|" #endif "-E|-r <file>|-w <file>|-v <file>]\n" - " [-c <chipname>] [-m [<vendor>:]<part>] [-l <file>]\n" + " [-c <chipname>] [-m [<vendor>:]<part>] [-l <file>] " + "[-o <file>]\n" " [-i <image>] [-p <programmername>[:<parameters>]]\n\n");
printf("Please note that the command line interface for flashrom has " @@ -72,6 +73,7 @@ "<file>\n" " -i | --image <name> only flash image <name> " "from flash layout\n" + " -o | --output <name> log to file <name>\n" " -L | --list-supported print supported devices\n" #if CONFIG_PRINT_WIKI == 1 " -z | --list-supported-wiki print supported devices " @@ -118,7 +120,7 @@ int operation_specified = 0; int i;
- static const char optstring[] = "r:Rw:v:nVEfc:m:l:i:p:Lzh"; + static const char optstring[] = "r:Rw:v:nVEfc:m:l:i:p:Lzho:"; static const struct option long_options[] = { {"read", 1, NULL, 'r'}, {"write", 1, NULL, 'w'}, @@ -136,6 +138,7 @@ {"programmer", 1, NULL, 'p'}, {"help", 0, NULL, 'h'}, {"version", 0, NULL, 'R'}, + {"output", 1, NULL, 'o'}, {NULL, 0, NULL, 0} };
@@ -307,14 +310,22 @@ cli_classic_usage(argv[0]); exit(0); break; + case 'o': + tempstr = strdup(optarg); +#ifdef STANDALONE + fprintf(stderr, "Log file not supported in standalone " + "mode. Aborting.\n"); +#else /* STANDALONE */ + if (open_logfile(tempstr)) +#endif /* STANDALONE */ + cli_classic_abort_usage(); + break; default: cli_classic_abort_usage(); break; } }
- /* FIXME: Print the actions flashrom will take. */ - if (list_supported) { print_supported(); exit(0); @@ -355,11 +366,21 @@ flash = NULL; }
+#ifndef STANDALONE + start_logging(); +#endif /* STANDALONE */ + + msg_gdbg("Command line:"); + for (i = 0; i < argc; i++) { + msg_gdbg(" %s", argv[i]); + } + msg_gdbg("\n"); + /* FIXME: Delay calibration should happen in programmer code. */ myusec_calibrate_delay();
if (programmer_init(pparam)) { - fprintf(stderr, "Error: Programmer initialization failed.\n"); + msg_perr("Error: Programmer initialization failed.\n"); exit(1); }
@@ -372,26 +393,31 @@ }
if (chipcount > 1) { - printf("Multiple flash chips were detected:"); + msg_cinfo("Multiple flash chips were detected:"); for (i = 0; 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"); programmer_shutdown(); exit(1); } 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) { - 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"); startchip = probe_flash(0, &flashes[0], 1); 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); programmer_shutdown(); exit(1); } - printf("Please note that forced reads most likely contain garbage.\n"); + msg_cinfo("Please note that forced reads most likely " + "contain garbage.\n"); return read_flash_to_file(&flashes[0], filename); } // FIXME: flash writes stay enabled! @@ -406,21 +432,21 @@ size = fill_flash->total_size * 1024; if (check_max_decode((buses_supported & fill_flash->bustype), size) && (!force)) { - fprintf(stderr, "Chip is too big for this programmer " - "(-V gives details). Use --force to override.\n"); + msg_cerr("Chip is too big for this programmer " + "(-V gives details). Use --force to override.\n"); programmer_shutdown(); return 1; }
if (!(read_it | write_it | verify_it | erase_it)) { - printf("No operations were specified.\n"); + msg_ginfo("No operations were specified.\n"); // FIXME: flash writes stay enabled! programmer_shutdown(); exit(0); }
if (!filename && !erase_it) { - printf("Error: No filename specified.\n"); + msg_gerr("Error: No filename specified.\n"); // FIXME: flash writes stay enabled! programmer_shutdown(); exit(1); Index: flashrom-logfile/flashrom.c =================================================================== --- flashrom-logfile/flashrom.c (Revision 1337) +++ flashrom-logfile/flashrom.c (Arbeitskopie) @@ -543,13 +543,18 @@
int programmer_shutdown(void) { + int ret = 0; /* Registering shutdown functions is no longer allowed. */ may_register_shutdown = 0; while (shutdown_fn_count > 0) { int i = --shutdown_fn_count; shutdown_fn[i].func(shutdown_fn[i].data); } - return programmer_table[programmer].shutdown(); + ret = programmer_table[programmer].shutdown(); +#ifndef STANDALONE + ret |= close_logfile(); +#endif + return ret; }
void *programmer_map_flash_region(const char *descr, unsigned long phys_addr, @@ -1649,37 +1654,37 @@ msg_ginfo(" on %s %s (%s)", osinfo.sysname, osinfo.release, osinfo.machine); #else - msg_ginfo(" on unknown machine"); + msg_gdbg(" on unknown machine"); #endif - msg_ginfo(", built with"); + msg_gdbg(", built with"); #if NEED_PCI == 1 #ifdef PCILIB_VERSION - msg_ginfo(" libpci %s,", PCILIB_VERSION); + msg_gdbg(" libpci %s,", PCILIB_VERSION); #else - msg_ginfo(" unknown PCI library,"); + msg_gdbg(" unknown PCI library,"); #endif #endif #ifdef __clang__ - msg_ginfo(" LLVM Clang"); + msg_gdbg(" LLVM Clang"); #ifdef __clang_version__ - msg_ginfo(" %s,", __clang_version__); + msg_gdbg(" %s,", __clang_version__); #else - msg_ginfo(" unknown version (before r102686),"); + msg_gdbg(" unknown version (before r102686),"); #endif #elif defined(__GNUC__) - msg_ginfo(" GCC"); + msg_gdbg(" GCC"); #ifdef __VERSION__ - msg_ginfo(" %s,", __VERSION__); + msg_gdbg(" %s,", __VERSION__); #else - msg_ginfo(" unknown version,"); + msg_gdbg(" unknown version,"); #endif #else - msg_ginfo(" unknown compiler,"); + msg_gdbg(" unknown compiler,"); #endif #if defined (__FLASHROM_LITTLE_ENDIAN__) - msg_ginfo(" little endian"); + msg_gdbg(" little endian"); #else - msg_ginfo(" big endian"); + msg_gdbg(" big endian"); #endif msg_ginfo("\n"); }