[flashrom] [PATCH] Move show_id to where it belongs.

Stefan Tauner stefan.tauner at student.tuwien.ac.at
Sat Aug 11 04:56:10 CEST 2012


And remove the unused force parameter.

Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
---
i forgot to move the declaration in the header, and i noticed that the
force parameter is useless because we use the global variable instead.
this should actually change eventually, but for now just kill the
parameter because it is doubtful that it will be part of the solution.

 cbtable.c    |   80 ++++++++++++++++++++++++++++++++++++++++++++++
 flashrom.c   |    2 +-
 layout.c     |  101 ----------------------------------------------------------
 programmer.h |    4 +--
 4 files changed, 82 insertions(+), 105 deletions(-)

diff --git a/cbtable.c b/cbtable.c
index b4fb809..dde12ac 100644
--- a/cbtable.c
+++ b/cbtable.c
@@ -25,6 +25,7 @@
 #include <unistd.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <ctype.h>
 #include <string.h>
 #include "flash.h"
 #include "programmer.h"
@@ -32,6 +33,9 @@
 
 char *lb_part = NULL, *lb_vendor = NULL;
 int partvendor_from_cbtable = 0;
+static char *def_name = "DEFAULT";
+static char *mainboard_vendor = NULL;
+static char *mainboard_part = NULL;
 
 /* Parse the [<vendor>:]<board> string specified by the user as part of
  * -p internal:mainboard=[<vendor>:]<board> and set lb_vendor and lb_part
@@ -56,6 +60,82 @@ void lb_vendor_dev_from_string(const char *boardstring)
 	free(tempstr);
 }
 
+int show_id(uint8_t *bios, int size)
+{
+	unsigned int *walk;
+	unsigned int mb_part_offset, mb_vendor_offset;
+	char *mb_part, *mb_vendor;
+
+	mainboard_vendor = def_name;
+	mainboard_part = def_name;
+
+	walk = (unsigned int *)(bios + size - 0x10);
+	walk--;
+
+	if ((*walk) == 0 || ((*walk) & 0x3ff) != 0) {
+		/* We might have an NVIDIA chipset BIOS which stores the ID information somewhere else. */
+		walk = (unsigned int *)(bios + size - 0x80);
+		walk--;
+	}
+
+	/*
+	 * Check if coreboot last image size is 0 or not a multiple of 1k or
+	 * bigger than the chip or if the pointers to vendor ID or mainboard ID
+	 * are outside the image of if the start of ID strings are nonsensical
+	 * (nonprintable and not \0).
+	 */
+	mb_part_offset = *(walk - 1);
+	mb_vendor_offset = *(walk - 2);
+	if ((*walk) == 0 || ((*walk) & 0x3ff) != 0 || (*walk) > size ||
+	    mb_part_offset > size || mb_vendor_offset > size) {
+		msg_pinfo("Flash image seems to be a legacy BIOS. Disabling coreboot-related checks.\n");
+		return 0;
+	}
+
+	mb_part = (char *)(bios + size - mb_part_offset);
+	mb_vendor = (char *)(bios + size - mb_vendor_offset);
+	if (!isprint((unsigned char)*mb_part) ||
+	    !isprint((unsigned char)*mb_vendor)) {
+		msg_pinfo("Flash image seems to have garbage in the ID location. Disabling checks.\n");
+		return 0;
+	}
+
+	msg_pdbg("coreboot last image size (not ROM size) is %d bytes.\n", *walk);
+
+	mainboard_part = strdup(mb_part);
+	mainboard_vendor = strdup(mb_vendor);
+	msg_pdbg("Manufacturer: %s\n", mainboard_vendor);
+	msg_pdbg("Mainboard ID: %s\n", mainboard_part);
+
+	/* If these are not set, the coreboot table was not found. */
+	if (!lb_vendor || !lb_part) {
+		msg_pinfo("Note: If the following flash access fails, try "
+			  "-p internal:mainboard=<vendor>:<mainboard>.\n");
+		return 0;
+	}
+
+	/* These comparisons are case insensitive to make things a little less user^Werror prone. */
+	if (!strcasecmp(mainboard_vendor, lb_vendor) &&
+	    !strcasecmp(mainboard_part, lb_part)) {
+		msg_pdbg("This firmware image matches this mainboard.\n");
+	} else {
+		if (force_boardmismatch) {
+			msg_pinfo("WARNING: This firmware image does not "
+			       "seem to fit to this machine - forcing it.\n");
+		} else {
+			msg_pinfo("ERROR: Your firmware image (%s:%s) does not appear to\n"
+				  "       be correct for the detected mainboard (%s:%s)\n\n"
+				  "Override with -p internal:boardmismatch=force to ignore the board name\n"
+				  "in the firmware image or override the detected mainboard with\n"
+				  "-p internal:mainboard=<vendor>:<mainboard>.\n\n",
+				  mainboard_vendor, mainboard_part, lb_vendor, lb_part);
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
 static unsigned long compute_checksum(void *addr, unsigned long length)
 {
 	uint8_t *ptr;
diff --git a/flashrom.c b/flashrom.c
index a925379..9544155 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -1815,7 +1815,7 @@ int doit(struct flashctx *flash, int force, const char *filename, int read_it,
 
 #if CONFIG_INTERNAL == 1
 		if (programmer == PROGRAMMER_INTERNAL)
-			if (show_id(newcontents, size, force)) {
+			if (show_id(newcontents, size)) {
 				ret = 1;
 				goto out;
 			}
diff --git a/layout.c b/layout.c
index 0512638..8663402 100644
--- a/layout.c
+++ b/layout.c
@@ -21,15 +21,10 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <ctype.h>
 #include <limits.h>
 #include "flash.h"
 #include "programmer.h"
 
-#if CONFIG_INTERNAL == 1
-char *mainboard_vendor = NULL;
-char *mainboard_part = NULL;
-#endif
 static int romimages = 0;
 
 #define MAX_ROMLAYOUT	32
@@ -49,102 +44,6 @@ static char *include_args[MAX_ROMLAYOUT];
 static int num_include_args = 0; /* the number of valid entries. */
 static romlayout_t rom_entries[MAX_ROMLAYOUT];
 
-#if CONFIG_INTERNAL == 1 /* FIXME: Move the whole block to cbtable.c? */
-static char *def_name = "DEFAULT";
-
-int show_id(uint8_t *bios, int size, int force)
-{
-	unsigned int *walk;
-	unsigned int mb_part_offset, mb_vendor_offset;
-	char *mb_part, *mb_vendor;
-
-	mainboard_vendor = def_name;
-	mainboard_part = def_name;
-
-	walk = (unsigned int *)(bios + size - 0x10);
-	walk--;
-
-	if ((*walk) == 0 || ((*walk) & 0x3ff) != 0) {
-		/* We might have an NVIDIA chipset BIOS which stores the ID
-		 * information at a different location.
-		 */
-		walk = (unsigned int *)(bios + size - 0x80);
-		walk--;
-	}
-
-	/*
-	 * Check if coreboot last image size is 0 or not a multiple of 1k or
-	 * bigger than the chip or if the pointers to vendor ID or mainboard ID
-	 * are outside the image of if the start of ID strings are nonsensical
-	 * (nonprintable and not \0).
-	 */
-	mb_part_offset = *(walk - 1);
-	mb_vendor_offset = *(walk - 2);
-	if ((*walk) == 0 || ((*walk) & 0x3ff) != 0 || (*walk) > size ||
-	    mb_part_offset > size || mb_vendor_offset > size) {
-		msg_pinfo("Flash image seems to be a legacy BIOS. "
-		          "Disabling coreboot-related checks.\n");
-		return 0;
-	}
-
-	mb_part = (char *)(bios + size - mb_part_offset);
-	mb_vendor = (char *)(bios + size - mb_vendor_offset);
-	if (!isprint((unsigned char)*mb_part) ||
-	    !isprint((unsigned char)*mb_vendor)) {
-		msg_pinfo("Flash image seems to have garbage in the ID location."
-		       " Disabling checks.\n");
-		return 0;
-	}
-
-	msg_pdbg("coreboot last image size "
-		     "(not ROM size) is %d bytes.\n", *walk);
-
-	mainboard_part = strdup(mb_part);
-	mainboard_vendor = strdup(mb_vendor);
-	msg_pdbg("Manufacturer: %s\n", mainboard_vendor);
-	msg_pdbg("Mainboard ID: %s\n", mainboard_part);
-
-	/*
-	 * If lb_vendor is not set, the coreboot table was
-	 * not found. Nor was -p internal:mainboard=VENDOR:PART specified.
-	 */
-	if (!lb_vendor || !lb_part) {
-		msg_pinfo("Note: If the following flash access fails, try "
-			  "-p internal:mainboard=<vendor>:<mainboard>.\n");
-		return 0;
-	}
-
-	/* These comparisons are case insensitive to make things
-	 * a little less user^Werror prone. 
-	 */
-	if (!strcasecmp(mainboard_vendor, lb_vendor) &&
-	    !strcasecmp(mainboard_part, lb_part)) {
-		msg_pdbg("This firmware image matches this mainboard.\n");
-	} else {
-		if (force_boardmismatch) {
-			msg_pinfo("WARNING: This firmware image does not "
-			       "seem to fit to this machine - forcing it.\n");
-		} else {
-			msg_pinfo("ERROR: Your firmware image (%s:%s) does not "
-				  "appear to\n"
-				  "       be correct for the detected "
-				  "mainboard (%s:%s)\n\n"
-				  "Override with -p internal:boardmismatch="
-				  "force to ignore the board name in the\n"
-				  "firmware image or override the detected "
-				  "mainboard with\n"
-				  "-p internal:mainboard=<vendor>:<mainboard>."
-				  "\n\n",
-				  mainboard_vendor, mainboard_part, lb_vendor,
-				  lb_part);
-			return 1;
-		}
-	}
-
-	return 0;
-}
-#endif
-
 #ifndef __LIBPAYLOAD__
 int read_romlayout(char *name)
 {
diff --git a/programmer.h b/programmer.h
index 723374d..7d06be3 100644
--- a/programmer.h
+++ b/programmer.h
@@ -274,6 +274,7 @@ void cleanup_cpu_msr(void);
 
 /* cbtable.c */
 void lb_vendor_dev_from_string(const char *boardstring);
+int show_id(uint8_t *bios, int size);
 int coreboot_init(void);
 extern char *lb_part, *lb_vendor;
 extern int partvendor_from_cbtable;
@@ -478,9 +479,6 @@ void check_chip_supported(const struct flashctx *flash);
 int check_max_decode(enum chipbustype buses, uint32_t size);
 char *extract_programmer_param(const char *param_name);
 
-/* layout.c */
-int show_id(uint8_t *bios, int size, int force);
-
 /* spi.c */
 enum spi_controller {
 	SPI_CONTROLLER_NONE,
-- 
Kind regards, Stefan Tauner





More information about the flashrom mailing list