>From 5c3670fbd34f8edf4f886101484f7cb09a0a96ae Mon Sep 17 00:00:00 2001
From: Stefan Tauner <stefan.tauner@student.tuwien.ac.at>
Date: Sun, 19 Jun 2011 14:39:37 +0200
Subject: [PATCH] squash! Add log file support to flashrom.

 - instead of the want_log variable "print" will always write to the log if a log file is open
   (indicated by logfile != NULL)
 - print_version is no longer executed "implicitly" before argument parsing
 - cli_classic uses the "goto cleanup"-pattern where it closes the log file

Signed-off-by: Stefan Tauner <stefan.tauner@student.tuwien.ac.at>
---
 cli_classic.c |   88 +++++++++++++++++++++++++++++++++++---------------------
 cli_output.c  |   27 ++++-------------
 flash.h       |    1 -
 flashrom.c    |    7 +---
 4 files changed, 64 insertions(+), 59 deletions(-)

diff --git a/cli_classic.c b/cli_classic.c
index ed36c85..71f8bec 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -108,6 +108,8 @@ int cli_classic(int argc, char *argv[])
 	int startchip = 0;
 	int chipcount = 0;
 	const char *name;
+	const char *log_name = NULL;
+	int ret = 0;
 	int namelen;
 	int opt;
 	int option_index = 0;
@@ -147,9 +149,6 @@ int cli_classic(int argc, char *argv[])
 	char *tempstr = NULL;
 	char *pparam = NULL;
 
-	print_version();
-	print_banner();
-
 	if (selfcheck())
 		exit(1);
 
@@ -293,7 +292,7 @@ int cli_classic(int argc, char *argv[])
 			}
 			break;
 		case 'R':
-			/* print_version() is always called during startup. */
+			print_version();
 			if (++operation_specified > 1) {
 				fprintf(stderr, "More than one operation "
 					"specified. Aborting.\n");
@@ -302,6 +301,7 @@ int cli_classic(int argc, char *argv[])
 			exit(0);
 			break;
 		case 'h':
+			print_version();
 			if (++operation_specified > 1) {
 				fprintf(stderr, "More than one operation "
 					"specified. Aborting.\n");
@@ -311,14 +311,18 @@ int cli_classic(int argc, char *argv[])
 			exit(0);
 			break;
 		case 'o':
-			tempstr = strdup(optarg);
 #ifdef STANDALONE
 			fprintf(stderr, "Log file not supported in standalone "
 				"mode. Aborting.\n");
+			cli_classic_abort_usage();
 #else /* STANDALONE */
-			if (open_logfile(tempstr))
-#endif /* STANDALONE */
+			log_name = strdup(optarg);
+			if (log_name == NULL || log_name[0] == '\0') {
+				fprintf(stderr,
+					"No log file name specified.\n");
 				cli_classic_abort_usage();
+			}
+#endif /* STANDALONE */
 			break;
 		default:
 			cli_classic_abort_usage();
@@ -326,18 +330,6 @@ int cli_classic(int argc, char *argv[])
 		}
 	}
 
-	if (list_supported) {
-		print_supported();
-		exit(0);
-	}
-
-#if CONFIG_PRINT_WIKI == 1
-	if (list_supported_wiki) {
-		print_supported_wiki();
-		exit(0);
-	}
-#endif
-
 	if (optind < argc) {
 		fprintf(stderr, "Error: Extra parameter found.\n");
 		cli_classic_abort_usage();
@@ -351,6 +343,26 @@ int cli_classic(int argc, char *argv[])
 	}
 #endif
 
+#if CONFIG_PRINT_WIKI == 1
+	if (list_supported_wiki) {
+		print_supported_wiki();
+		return 0;
+	}
+#endif
+
+#ifndef STANDALONE
+	if (open_logfile(log_name))
+		return 1;
+#endif /* !STANDALONE */
+
+	print_version();
+	print_banner();
+
+	if (list_supported) {
+		print_supported();
+		goto cleanup;
+	}
+
 	if (chip_to_probe) {
 		for (flash = flashchips; flash && flash->name; flash++)
 			if (!strcmp(flash->name, chip_to_probe))
@@ -360,16 +372,13 @@ int cli_classic(int argc, char *argv[])
 				chip_to_probe);
 			printf("Run flashrom -L to view the hardware supported "
 				"in this flashrom version.\n");
-			exit(1);
+			ret = 1;
+			goto cleanup;
 		}
 		/* Clean up after the check. */
 		flash = NULL;
 	}
 
-#ifndef STANDALONE
-	start_logging();
-#endif /* STANDALONE */
-
 	msg_gdbg("Command line:");
 	for (i = 0; i < argc; i++) {
 		msg_gdbg(" %s", argv[i]);
@@ -382,7 +391,8 @@ int cli_classic(int argc, char *argv[])
 	if (programmer_init(pparam)) {
 		msg_perr("Error: Programmer initialization failed.\n");
 		programmer_shutdown();
-		exit(1);
+		ret = 1;
+		goto cleanup;
 	}
 
 	for (i = 0; i < ARRAY_SIZE(flashes); i++) {
@@ -400,7 +410,8 @@ int cli_classic(int argc, char *argv[])
 		msg_cinfo("\nPlease specify which chip to use with the -c "
 			  "<chipname> option.\n");
 		programmer_shutdown();
-		exit(1);
+		ret = 1;
+		goto cleanup;
 	} else if (!chipcount) {
 		msg_cinfo("No EEPROM/flash device found.\n");
 		if (!force || !chip_to_probe) {
@@ -415,15 +426,18 @@ int cli_classic(int argc, char *argv[])
 				msg_cinfo("Probing for flash chip '%s' failed."
 					  "\n", chip_to_probe);
 				programmer_shutdown();
-				exit(1);
+				ret = 1;
+				goto cleanup;
 			}
 			msg_cinfo("Please note that forced reads most likely "
 				  "contain garbage.\n");
-			return read_flash_to_file(&flashes[0], filename);
+			ret |= read_flash_to_file(&flashes[0], filename);
+			goto cleanup;
 		}
 		// FIXME: flash writes stay enabled!
 		programmer_shutdown();
-		exit(1);
+		ret = 1;
+		goto cleanup;
 	}
 
 	fill_flash = &flashes[0];
@@ -436,21 +450,23 @@ int cli_classic(int argc, char *argv[])
 		msg_cerr("Chip is too big for this programmer "
 			 "(-V gives details). Use --force to override.\n");
 		programmer_shutdown();
-		return 1;
+		ret = 1;
+		goto cleanup;
 	}
 
 	if (!(read_it | write_it | verify_it | erase_it)) {
 		msg_ginfo("No operations were specified.\n");
 		// FIXME: flash writes stay enabled!
 		programmer_shutdown();
-		exit(0);
+		goto cleanup;
 	}
 
 	if (!filename && !erase_it) {
 		msg_gerr("Error: No filename specified.\n");
 		// FIXME: flash writes stay enabled!
 		programmer_shutdown();
-		exit(1);
+		ret = 1;
+		goto cleanup;
 	}
 
 	/* Always verify write operations unless -n is used. */
@@ -462,5 +478,11 @@ int cli_classic(int argc, char *argv[])
 	 * 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);
+
+cleanup:
+#ifndef STANDALONE
+	ret |= close_logfile();
+#endif
+	return ret;
 }
diff --git a/cli_output.c b/cli_output.c
index 7d812d6..88738bf 100644
--- a/cli_output.c
+++ b/cli_output.c
@@ -24,39 +24,26 @@
 #include "flash.h"
 
 static FILE *logfile = NULL;
-static int want_log = 0;
 
 #ifndef STANDALONE
 int close_logfile(void)
 {
-	if (logfile && fclose(logfile))
-		return 1;
-	return 0;
+	int ret = 0;
+	if (logfile) {
+		ret |= fclose(logfile);
+		logfile = NULL;
+	} // else ret = 1;?
+	return ret;
 }
 
 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, ...)
@@ -109,7 +96,7 @@ int print(int type, const char *fmt, ...)
 		ret = vfprintf(output_type, fmt, ap);
 		va_end(ap);
 	}
-	if (want_file && logfile && want_log) {
+	if (want_file && logfile) {
 		va_start(ap, fmt);
 		ret = vfprintf(logfile, fmt, ap);
 		va_end(ap);
diff --git a/flash.h b/flash.h
index 6401b81..9d71229 100644
--- a/flash.h
+++ b/flash.h
@@ -232,7 +232,6 @@ int write_buf_to_file(unsigned char *buf, unsigned long size, char *filename);
 #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. */
diff --git a/flashrom.c b/flashrom.c
index 55f4942..5daaaaf 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -533,9 +533,6 @@ int programmer_shutdown(void)
 		int i = --shutdown_fn_count;
 		ret |= shutdown_fn[i].func(shutdown_fn[i].data);
 	}
-#ifndef STANDALONE
-	ret |= close_logfile();
-#endif
 	return ret;
 }
 
@@ -1682,8 +1679,8 @@ void print_version(void)
 
 void print_banner(void)
 {
-	msg_ginfo("flashrom is free software, get the source code at "
-		    "http://www.flashrom.org\n");
+	msg_ginfo("flashrom is free software. Get the source code at "
+		  "http://www.flashrom.org\n");
 	msg_ginfo("\n");
 }
 
-- 
1.7.1

