[flashrom] [PATCH] Rework extract_programmer_param to remove exit call

Niklas Söderlund niso at kth.se
Fri Oct 19 02:03:21 CEST 2012


- Utilise the reentrant strtok_r function to reduce complexity in
  extract_programmer_param.
- Remove exit call in extract_programmer_param and introduce return code
  to indicate pass/fail. This pushes error handling to the caller.
- Update all callers to do proper error handling.
- Push checking of parameter key=value format checking to
  extract_programmer_param. Not all callers checked for this and this
  greatly reduces complexity at the caller.

Signed-off-by: Niklas Söderlund <niso at kth.se>
---
 buspirate_spi.c  |  16 +++-----
 chipset_enable.c |  14 +++----
 dediprog.c       |   5 +--
 dummyflasher.c   |  32 ++++++++--------
 flash.h          |   1 -
 flashrom.c       | 110 ++++++++++++++++++++++++++-----------------------------
 ft2232_spi.c     |  29 ++++++++-------
 ichspi.c         |  56 ++++++++++++----------------
 internal.c       |  70 ++++++++++++++---------------------
 it87spi.c        |   5 +--
 linux_spi.c      |   9 ++---
 ogp_spi.c        |   8 ++--
 pcidev.c         |   6 +--
 pony_spi.c       |  31 +++++++---------
 programmer.h     |   3 +-
 rayer_spi.c      |  12 +++---
 serprog.c        |  41 ++++++++-------------
 17 files changed, 194 insertions(+), 254 deletions(-)

diff --git a/buspirate_spi.c b/buspirate_spi.c
index 054b4ff..af37628 100644
--- a/buspirate_spi.c
+++ b/buspirate_spi.c
@@ -208,28 +208,22 @@ int buspirate_spi_init(void)
 	int ret = 0;
 	int i;
 
-	dev = extract_programmer_param("dev");
-	if (dev && !strlen(dev)) {
-		free(dev);
-		dev = NULL;
-	}
-	if (!dev) {
+	if (extract_programmer_param("dev", &dev)) {
 		msg_perr("No serial device given. Use flashrom -p buspirate_spi:dev=/dev/ttyUSB0\n");
 		return 1;
 	}
 
-	speed = extract_programmer_param("spispeed");
-	if (speed) {
+	if (!extract_programmer_param("spispeed", &speed)) {
 		for (i = 0; spispeeds[i].name; i++)
-			if (!strncasecmp(spispeeds[i].name, speed,
-			    strlen(spispeeds[i].name))) {
+			if (!strncasecmp(spispeeds[i].name, speed, strlen(spispeeds[i].name))) {
 				spispeed = spispeeds[i].speed;
 				break;
 			}
 		if (!spispeeds[i].name)
 			msg_perr("Invalid SPI speed, using default.\n");
+
+		free(speed);
 	}
-	free(speed);
 
 	/* Default buffer size is 19: 16 bytes data, 3 bytes control. */
 #define DEFAULT_BUFSIZE (16 + 3)
diff --git a/chipset_enable.c b/chipset_enable.c
index 0873b4e..bddf1c7 100644
--- a/chipset_enable.c
+++ b/chipset_enable.c
@@ -350,8 +350,7 @@ static int enable_flash_ich_dc(struct pci_dev *dev, const char *name)
 	int max_decode_fwh_idsel = 0, max_decode_fwh_decode = 0;
 	int contiguous = 1;
 
-	idsel = extract_programmer_param("fwh_idsel");
-	if (idsel && strlen(idsel)) {
+	if (!extract_programmer_param("fwh_idsel", &idsel)) {
 		uint64_t fwh_idsel_old, fwh_idsel;
 		errno = 0;
 		/* Base 16, nothing else makes sense. */
@@ -359,12 +358,14 @@ static int enable_flash_ich_dc(struct pci_dev *dev, const char *name)
 		if (errno) {
 			msg_perr("Error: fwh_idsel= specified, but value could "
 				 "not be converted.\n");
-			goto idsel_garbage_out;
+			free(idsel);
+			return ERROR_FATAL;
 		}
 		if (fwh_idsel & 0xffff000000000000ULL) {
 			msg_perr("Error: fwh_idsel= specified, but value had "
 				 "unused bits set.\n");
-			goto idsel_garbage_out;
+			free(idsel);
+			return ERROR_FATAL;
 		}
 		fwh_idsel_old = pci_read_long(dev, 0xd0);
 		fwh_idsel_old <<= 16;
@@ -375,13 +376,8 @@ static int enable_flash_ich_dc(struct pci_dev *dev, const char *name)
 		rpci_write_long(dev, 0xd0, (fwh_idsel >> 16) & 0xffffffff);
 		rpci_write_word(dev, 0xd4, fwh_idsel & 0xffff);
 		/* FIXME: Decode settings are not changed. */
-	} else if (idsel) {
-		msg_perr("Error: fwh_idsel= specified, but no value given.\n");
-idsel_garbage_out:
 		free(idsel);
-		return ERROR_FATAL;
 	}
-	free(idsel);
 
 	/* Ignore all legacy ranges below 1 MB.
 	 * We currently only support flashing the chip which responds to
diff --git a/dediprog.c b/dediprog.c
index a81cf83..0986853 100644
--- a/dediprog.c
+++ b/dediprog.c
@@ -783,8 +783,7 @@ int dediprog_init(void)
 
 	msg_pspew("%s\n", __func__);
 
-	voltage = extract_programmer_param("voltage");
-	if (voltage) {
+	if (!extract_programmer_param("voltage", &voltage)) {
 		millivolt = parse_voltage(voltage);
 		free(voltage);
 		if (millivolt < 0)
@@ -821,7 +820,7 @@ int dediprog_init(void)
 		return 1;
 	}
 	dediprog_endpoint = 2;
-	
+
 	if (register_shutdown(dediprog_shutdown, NULL))
 		return 1;
 
diff --git a/dummyflasher.c b/dummyflasher.c
index 655b678..580166e 100644
--- a/dummyflasher.c
+++ b/dummyflasher.c
@@ -174,10 +174,14 @@ int dummy_init(void)
 
 	msg_pspew("%s\n", __func__);
 
-	bustext = extract_programmer_param("bus");
-	msg_pdbg("Requested buses are: %s\n", bustext ? bustext : "default");
-	if (!bustext)
+
+	if (!extract_programmer_param("bus", &bustext))
+		msg_pdbg("Requested buses are: %s\n", bustext);
+	else {
+		msg_pdbg("Requested buses are: default\n");
 		bustext = strdup("parallel+lpc+fwh+spi");
+	}
+
 	/* Convert the parameters to lowercase. */
 	tolower_string(bustext);
 
@@ -202,8 +206,7 @@ int dummy_init(void)
 		msg_pdbg("Support for all flash bus types disabled.\n");
 	free(bustext);
 
-	tmp = extract_programmer_param("spi_write_256_chunksize");
-	if (tmp) {
+	if (!extract_programmer_param("spi_write_256_chunksize", &tmp)) {
 		spi_write_256_chunksize = atoi(tmp);
 		free(tmp);
 		if (spi_write_256_chunksize < 1) {
@@ -212,8 +215,7 @@ int dummy_init(void)
 		}
 	}
 
-	tmp = extract_programmer_param("spi_blacklist");
-	if (tmp) {
+	if (!extract_programmer_param("spi_blacklist", &tmp)) {
 		i = strlen(tmp);
 		if (!strncmp(tmp, "0x", 2)) {
 			i -= 2;
@@ -245,11 +247,10 @@ int dummy_init(void)
 		for (i = 0; i < spi_blacklist_size; i++)
 			msg_pdbg("%02x ", spi_blacklist[i]);
 		msg_pdbg(", size %i\n", spi_blacklist_size);
+		free(tmp);
 	}
-	free(tmp);
 
-	tmp = extract_programmer_param("spi_ignorelist");
-	if (tmp) {
+	if (!extract_programmer_param("spi_ignorelist", &tmp)) {
 		i = strlen(tmp);
 		if (!strncmp(tmp, "0x", 2)) {
 			i -= 2;
@@ -281,12 +282,11 @@ int dummy_init(void)
 		for (i = 0; i < spi_ignorelist_size; i++)
 			msg_pdbg("%02x ", spi_ignorelist[i]);
 		msg_pdbg(", size %i\n", spi_ignorelist_size);
+		free(tmp);
 	}
-	free(tmp);
 
 #if EMULATE_CHIP
-	tmp = extract_programmer_param("emulate");
-	if (!tmp) {
+	if (extract_programmer_param("emulate", &tmp)) {
 		msg_pdbg("Not emulating any flash chip.\n");
 		/* Nothing else to do. */
 		goto dummy_init_out;
@@ -358,8 +358,7 @@ int dummy_init(void)
 	}
 
 #ifdef EMULATE_SPI_CHIP
-	status = extract_programmer_param("spi_status");
-	if (status) {
+	if (!extract_programmer_param("spi_status", &status)) {
 		char *endptr;
 		errno = 0;
 		emu_status = strtoul(status, &endptr, 0);
@@ -377,8 +376,7 @@ int dummy_init(void)
 	msg_pdbg("Filling fake flash chip with 0xff, size %i\n", emu_chip_size);
 	memset(flashchip_contents, 0xff, emu_chip_size);
 
-	emu_persistent_image = extract_programmer_param("image");
-	if (!emu_persistent_image) {
+	if (extract_programmer_param("image", &emu_persistent_image)) {
 		/* Nothing else to do. */
 		goto dummy_init_out;
 	}
diff --git a/flash.h b/flash.h
index 0bb6c2b..4bb2800 100644
--- a/flash.h
+++ b/flash.h
@@ -230,7 +230,6 @@ int read_flash_to_file(struct flashctx *flash, const char *filename);
 int min(int a, int b);
 int max(int a, int b);
 void tolower_string(char *str);
-char *extract_param(char **haystack, const char *needle, const char *delim);
 int verify_range(struct flashctx *flash, uint8_t *cmpbuf, unsigned int start, unsigned int len, const char *message);
 int need_erase(uint8_t *have, uint8_t *want, unsigned int len, enum write_granularity gran);
 char *strcat_realloc(char *dest, const char *src);
diff --git a/flashrom.c b/flashrom.c
index a887e3b..6c3f93e 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -465,71 +465,65 @@ char *strcat_realloc(char *dest, const char *src)
 	return dest;
 }
 
-/* This is a somewhat hacked function similar in some ways to strtok().
- * It will look for needle with a subsequent '=' in haystack, return a copy of
- * needle and remove everything from the first occurrence of needle to the next
- * delimiter from haystack.
- */
-char *extract_param(char **haystack, const char *needle, const char *delim)
+static int extract_param(char *haystack, const char const *needle, char **value)
 {
-	char *param_pos, *opt_pos, *rest;
-	char *opt = NULL;
-	int optlen;
-	int needlelen;
-
-	needlelen = strlen(needle);
-	if (!needlelen) {
-		msg_gerr("%s: empty needle! Please report a bug at "
-			 "flashrom at flashrom.org\n", __func__);
-		return NULL;
-	}
-	/* No programmer parameters given. */
-	if (*haystack == NULL)
-		return NULL;
-	param_pos = strstr(*haystack, needle);
-	do {
-		if (!param_pos)
-			return NULL;
-		/* Needle followed by '='? */
-		if (param_pos[needlelen] == '=') {
-			
-			/* Beginning of the string? */
-			if (param_pos == *haystack)
-				break;
-			/* After a delimiter? */
-			if (strchr(delim, *(param_pos - 1)))
-				break;
-		}
-		/* Continue searching. */
-		param_pos++;
-		param_pos = strstr(param_pos, needle);
-	} while (1);
-
-	if (param_pos) {
-		/* Get the string after needle and '='. */
-		opt_pos = param_pos + needlelen + 1;
-		optlen = strcspn(opt_pos, delim);
-		/* Return an empty string if the parameter was empty. */
-		opt = malloc(optlen + 1);
-		if (!opt) {
-			msg_gerr("Out of memory!\n");
-			exit(1);
+	char *pair, *pairptr;
+	char *keyptr, *tmp;
+
+	pair = strtok_r(haystack, ",", &pairptr);
+	while (pair) {
+		tmp = strtok_r(pair, "=", &keyptr);
+		if (!strcmp(tmp, needle)) {
+			tmp = strtok_r(NULL, "=", &keyptr);
+
+			if (!tmp) {
+				msg_perr("Missing argument for %s.\n", needle);
+				return 1;
+			}
+
+			*value = strdup(tmp);
+
+			if (!*value) {
+				msg_gerr("Out of memory!\n");
+				return 1;
+			}
+
+			return 0;
 		}
-		strncpy(opt, opt_pos, optlen);
-		opt[optlen] = '\0';
-		rest = opt_pos + optlen;
-		/* Skip all delimiters after the current parameter. */
-		rest += strspn(rest, delim);
-		memmove(param_pos, rest, strlen(rest) + 1);
-		/* We could shrink haystack, but the effort is not worth it. */
+		pair = strtok_r(NULL, ",", &pairptr);
 	}
 
-	return opt;
+	return 1;
 }
 
-char *extract_programmer_param(const char *param_name)
+int extract_programmer_param(const char const *param_name, char **value)
 {
-	return extract_param(&programmer_param, param_name, ",");
+	char *scratch;
+	int ret;
+
+	*value = NULL;
+
+	if (!strlen(param_name)) {
+		msg_gerr("%s: no param name! Please report a bug at flashrom at flashrom.org\n", __func__);
+		return 1;
+	}
+
+	/* No programmer parameters given. */
+	if (programmer_param == NULL)
+		return 1;
+
+	/* Alloc scratch string */
+	scratch = strdup(programmer_param);
+	if (!scratch) {
+		msg_gerr("Out of memory!\n");
+		return 1;
+	}
+
+	ret = extract_param(scratch, param_name, value);
+
+	free(scratch);
+
+	return ret;
 }
 
 /* Returns the number of well-defined erasers for a chip. */
diff --git a/ft2232_spi.c b/ft2232_spi.c
index 31a6c5c..49efa69 100644
--- a/ft2232_spi.c
+++ b/ft2232_spi.c
@@ -184,8 +184,7 @@ int ft2232_spi_init(void)
 	char *arg;
 	double mpsse_clk;
 
-	arg = extract_programmer_param("type");
-	if (arg) {
+	if (!extract_programmer_param("type", &arg)) {
 		if (!strcasecmp(arg, "2232H")) {
 			ft2232_type = FTDI_FT2232H_PID;
 			channel_count = 2;
@@ -243,11 +242,10 @@ int ft2232_spi_init(void)
 			free(arg);
 			return -1;
 		}
+		free(arg);
 	}
-	free(arg);
 
-	arg = extract_programmer_param("port");
-	if (arg) {
+	if (!extract_programmer_param("port", &arg)) {
 		switch (toupper((unsigned char)*arg)) {
 		case 'A':
 			ft2232_interface = INTERFACE_A;
@@ -279,21 +277,20 @@ int ft2232_spi_init(void)
 	}
 	free(arg);
 
-	arg = extract_programmer_param("divisor");
-	if (arg && strlen(arg)) {
+	if (!extract_programmer_param("divisor", &arg)) {
 		unsigned int temp = 0;
 		char *endptr;
 		temp = strtoul(arg, &endptr, 10);
 		if (*endptr || temp < 2 || temp > 131072 || temp & 0x1) {
 			msg_perr("Error: Invalid SPI frequency divisor specified: \"%s\".\n"
-				 "Valid are even values between 2 and 131072.\n", arg);
+					"Valid are even values between 2 and 131072.\n", arg);
 			free(arg);
 			return -2;
 		} else {
 			divisor = (uint32_t)temp;
 		}
+		free(arg);
 	}
-	free(arg);
 
 	msg_pdbg("Using device type %s %s ",
 		 get_ft2232_vendorname(ft2232_vid, ft2232_type),
@@ -312,12 +309,16 @@ int ft2232_spi_init(void)
 		msg_perr("Unable to select channel (%s).\n", ftdi_get_error_string(ftdic));
 	}
 
-	arg = extract_programmer_param("serial");
-	f = ftdi_usb_open_desc(ftdic, ft2232_vid, ft2232_type, NULL, arg);
-	free(arg);
+	if (!extract_programmer_param("serial", &arg)) {
+		f = ftdi_usb_open_desc(ftdic, ft2232_vid, ft2232_type, NULL, arg);
+		free(arg);
 
-	if (f < 0 && f != -5) {
-		msg_perr("Unable to open FTDI device: %d (%s).\n", f, ftdi_get_error_string(ftdic));
+		if (f < 0 && f != -5) {
+			msg_perr("Unable to open FTDI device: %d (%s).\n", f, ftdi_get_error_string(ftdic));
+			return -4;
+		}
+	} else {
+		msg_perr("No serial paramater specified.\n");
 		return -4;
 	}
 
diff --git a/ichspi.c b/ichspi.c
index 8dd1893..61f63b3 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1627,43 +1627,33 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb,
 		break;
 	case CHIPSET_ICH8:
 	default:		/* Future version might behave the same */
-		arg = extract_programmer_param("ich_spi_mode");
-		if (arg && !strcmp(arg, "hwseq")) {
-			ich_spi_mode = ich_hwseq;
-			msg_pspew("user selected hwseq\n");
-		} else if (arg && !strcmp(arg, "swseq")) {
-			ich_spi_mode = ich_swseq;
-			msg_pspew("user selected swseq\n");
-		} else if (arg && !strcmp(arg, "auto")) {
-			msg_pspew("user selected auto\n");
-			ich_spi_mode = ich_auto;
-		} else if (arg && !strlen(arg)) {
-			msg_perr("Missing argument for ich_spi_mode.\n");
-			free(arg);
-			return ERROR_FATAL;
-		} else if (arg) {
-			msg_perr("Unknown argument for ich_spi_mode: %s\n",
-				 arg);
+		if (!extract_programmer_param("ich_spi_mode", &arg)) {
+			if (!strcmp(arg, "hwseq"))
+				ich_spi_mode = ich_hwseq;
+			else if (!strcmp(arg, "swseq"))
+				ich_spi_mode = ich_swseq;
+			else if (!strcmp(arg, "auto"))
+				ich_spi_mode = ich_auto;
+			else {
+				msg_perr("Unknown argument for ich_spi_mode: %s\n", arg);
+				free(arg);
+				return ERROR_FATAL;
+			}
+			msg_pspew("user selected %s\n", arg);
 			free(arg);
-			return ERROR_FATAL;
 		}
-		free(arg);
-
-		arg = extract_programmer_param("ich_spi_force");
-		if (arg && !strcmp(arg, "yes")) {
-			ich_spi_force = 1;
-			msg_pspew("ich_spi_force enabled.\n");
-		} else if (arg && !strlen(arg)) {
-			msg_perr("Missing argument for ich_spi_force.\n");
-			free(arg);
-			return ERROR_FATAL;
-		} else if (arg) {
-			msg_perr("Unknown argument for ich_spi_force: \"%s\" "
-				 "(not \"yes\").\n", arg);
+
+		if (!extract_programmer_param("ich_spi_force", &arg)) {
+			if (!strcmp(arg, "yes")) {
+				ich_spi_force = 1;
+				msg_pspew("ich_spi_force enabled.\n");
+			} else {
+				msg_perr("Unknown argument for ich_spi_force: \"%s\" (not \"yes\").\n", arg);
+				free(arg);
+				return ERROR_FATAL;
+			}
 			free(arg);
-			return ERROR_FATAL;
 		}
-		free(arg);
 
 		tmp2 = mmio_readw(ich_spibar + ICH9_REG_HSFS);
 		msg_pdbg("0x04: 0x%04x (HSFS)\n", tmp2);
diff --git a/internal.c b/internal.c
index eda4d59..80bb794 100644
--- a/internal.c
+++ b/internal.c
@@ -176,62 +176,48 @@ int internal_init(void)
 	const char *cb_model = NULL;
 	char *arg;
 
-	arg = extract_programmer_param("boardenable");
-	if (arg && !strcmp(arg,"force")) {
-		force_boardenable = 1;
-	} else if (arg && !strlen(arg)) {
-		msg_perr("Missing argument for boardenable.\n");
-		free(arg);
-		return 1;
-	} else if (arg) {
-		msg_perr("Unknown argument for boardenable: %s\n", arg);
+	if (!extract_programmer_param("boardenable", &arg)) {
+		if (!strcmp(arg,"force")) {
+			force_boardenable = 1;
+		} else {
+			msg_perr("Unknown argument for boardenable: %s\n", arg);
+			free(arg);
+			return 1;
+		}
 		free(arg);
-		return 1;
 	}
-	free(arg);
 
-	arg = extract_programmer_param("boardmismatch");
-	if (arg && !strcmp(arg,"force")) {
-		force_boardmismatch = 1;
-	} else if (arg && !strlen(arg)) {
-		msg_perr("Missing argument for boardmismatch.\n");
-		free(arg);
-		return 1;
-	} else if (arg) {
-		msg_perr("Unknown argument for boardmismatch: %s\n", arg);
+	if (!extract_programmer_param("boardmismatch", &arg)) {
+		if (!strcmp(arg,"force")) {
+			force_boardmismatch = 1;
+		} else {
+			msg_perr("Unknown argument for boardmismatch: %s\n", arg);
+			free(arg);
+			return 1;
+		}
 		free(arg);
-		return 1;
 	}
-	free(arg);
-
-	arg = extract_programmer_param("laptop");
-	if (arg && !strcmp(arg, "force_I_want_a_brick"))
-		force_laptop = 1;
-	else if (arg && !strcmp(arg, "this_is_not_a_laptop"))
-		not_a_laptop = 1;
-	else if (arg && !strlen(arg)) {
-		msg_perr("Missing argument for laptop.\n");
-		free(arg);
-		return 1;
-	} else if (arg) {
-		msg_perr("Unknown argument for laptop: %s\n", arg);
+
+	if (!extract_programmer_param("laptop", &arg)) {
+		if (!strcmp(arg, "force_I_want_a_brick"))
+			force_laptop = 1;
+		else if (!strcmp(arg, "this_is_not_a_laptop"))
+			not_a_laptop = 1;
+		else {
+			msg_perr("Unknown argument for laptop: %s\n", arg);
+			free(arg);
+			return 1;
+		}
 		free(arg);
-		return 1;
 	}
-	free(arg);
 
-	arg = extract_programmer_param("mainboard");
-	if (arg && strlen(arg)) {
+	if (!extract_programmer_param("mainboard", &arg)) {
 		if (board_parse_parameter(arg, &board_vendor, &board_model)) {
 			free(arg);
 			return 1;
 		}
-	} else if (arg && !strlen(arg)) {
-		msg_perr("Missing argument for mainboard.\n");
 		free(arg);
-		return 1;
 	}
-	free(arg);
 
 	if (rget_io_perms())
 		return 1;
diff --git a/it87spi.c b/it87spi.c
index a35ddc0..a85d37b 100644
--- a/it87spi.c
+++ b/it87spi.c
@@ -166,8 +166,7 @@ static uint16_t it87spi_probe(uint16_t port)
 	flashport |= sio_read(port, 0x65);
 	msg_pdbg("Serial flash port 0x%04x\n", flashport);
 	/* Non-default port requested? */
-	portpos = extract_programmer_param("it87spiport");
-	if (portpos) {
+	if (!extract_programmer_param("it87spiport", &portpos)) {
 		char *endptr = NULL;
 		unsigned long forced_flashport;
 		forced_flashport = strtoul(portpos, &endptr, 0);
@@ -192,8 +191,8 @@ static uint16_t it87spi_probe(uint16_t port)
 			sio_write(port, 0x64, (flashport >> 8));
 			sio_write(port, 0x65, (flashport & 0xff));
 		}
+		free(portpos);
 	}
-	free(portpos);
 	exit_conf_mode_ite(port);
 	it8716f_flashport = flashport;
 	if (internal_buses_supported & BUS_SPI)
diff --git a/linux_spi.c b/linux_spi.c
index 2f46463..2221ba0 100644
--- a/linux_spi.c
+++ b/linux_spi.c
@@ -66,15 +66,12 @@ int linux_spi_init(void)
 	const uint8_t mode = SPI_MODE_0;
 	const uint8_t bits = 8;
 
-	dev = extract_programmer_param("dev");
-	if (!dev || !strlen(dev)) {
-		msg_perr("No SPI device given. Use flashrom -p "
-			 "linux_spi:dev=/dev/spidevX.Y\n");
+	if (extract_programmer_param("dev", &dev)) {
+		msg_perr("No SPI device given. Use flashrom -p linux_spi:dev=/dev/spidevX.Y\n");
 		return 1;
 	}
 
-	p = extract_programmer_param("speed");
-	if (p && strlen(p)) {
+	if (!extract_programmer_param("speed", &p)) {
 		speed = (uint32_t)strtoul(p, &endp, 10) * 1024;
 		if (p == endp) {
 			msg_perr("%s: invalid clock: %s kHz\n", __func__, p);
diff --git a/ogp_spi.c b/ogp_spi.c
index 25550b5..9b596bf 100644
--- a/ogp_spi.c
+++ b/ogp_spi.c
@@ -109,13 +109,13 @@ int ogp_spi_init(void)
 {
 	char *type;
 
-	type = extract_programmer_param("rom");
-
-	if (!type) {
+	if (extract_programmer_param("rom", &type)) {
 		msg_perr("Please use flashrom -p ogp_spi:rom=... to specify "
 			 "which flashchip you want to access.\n");
 		return 1;
-	} else if (!strcasecmp(type, "bprom") || !strcasecmp(type, "bios")) {
+	}
+
+	if (!strcasecmp(type, "bprom") || !strcasecmp(type, "bios")) {
 		ogp_reg_sel  = OGA1_XP10_BPROM_REG_SEL;
 		ogp_reg_siso = OGA1_XP10_BPROM_SI;
 		ogp_reg__ce  = OGA1_XP10_BPROM_CE_BAR;
diff --git a/pcidev.c b/pcidev.c
index f2cbbac..37319c9 100644
--- a/pcidev.c
+++ b/pcidev.c
@@ -170,14 +170,14 @@ uintptr_t pcidev_init(int bar, const struct pcidev_status *devs)
 	pci_filter_init(pacc, &filter);
 
 	/* Filter by bb:dd.f (if supplied by the user). */
-	pcidev_bdf = extract_programmer_param("pci");
-	if (pcidev_bdf != NULL) {
+	if (!extract_programmer_param("pci", &pcidev_bdf)) {
 		if ((msg = pci_filter_parse_slot(&filter, pcidev_bdf))) {
 			msg_perr("Error: %s\n", msg);
+			free(pcidev_bdf);
 			return 0;
 		}
+		free(pcidev_bdf);
 	}
-	free(pcidev_bdf);
 
 	for (dev = pacc->devices; dev; dev = dev->next) {
 		if (pci_filter_match(&filter, dev)) {
diff --git a/pony_spi.c b/pony_spi.c
index 7875200..7b9e72e 100644
--- a/pony_spi.c
+++ b/pony_spi.c
@@ -113,16 +113,15 @@ int pony_spi_init(void)
 	int have_prog = 0;
 
 	/* The parameter is in format "dev=/dev/device,type=serbang" */
-	arg = extract_programmer_param("dev");
-	if (arg && strlen(arg)) {
+	if (!extract_programmer_param("dev", &arg)) {
 		sp_fd = sp_openserport(arg, 9600);
 		if (sp_fd < 0) {
 			free(arg);
 			return 1;
 		}
 		have_device++;
+		free(arg);
 	}
-	free(arg);
 
 	if (!have_device) {
 		msg_perr("Error: No valid device specified.\n"
@@ -130,22 +129,20 @@ int pony_spi_init(void)
 		return 1;
 	}
 
-	arg = extract_programmer_param("type");
-	if (arg && !strcasecmp(arg, "serbang")) {
-		type = TYPE_SERBANG;
-	} else if (arg && !strcasecmp(arg, "si_prog")) {
-		type = TYPE_SI_PROG;
-	} else if (arg && !strcasecmp( arg, "ajawe")) {
-		type = TYPE_AJAWE;
-	} else if (arg && !strlen(arg)) {
-		msg_perr("Error: Missing argument for programmer type.\n");
-		free(arg);
-	} else if (arg){
-		msg_perr("Error: Invalid programmer type specified.\n");
+	if (!extract_programmer_param("type", &arg)) {
+		if (!strcasecmp(arg, "serbang")) {
+			type = TYPE_SERBANG;
+		} else if (!strcasecmp(arg, "si_prog")) {
+			type = TYPE_SI_PROG;
+		} else if (!strcasecmp(arg, "ajawe")) {
+			type = TYPE_AJAWE;
+		} else {
+			msg_perr("Error: Invalid programmer type specified.\n");
+			free(arg);
+			return 1;
+		}
 		free(arg);
-		return 1;
 	}
-	free(arg);
 
 	/*
 	 * Configure the serial port pins, depending on the used programmer.
diff --git a/programmer.h b/programmer.h
index dedec67..93c3f93 100644
--- a/programmer.h
+++ b/programmer.h
@@ -475,7 +475,8 @@ extern int programmer_may_write;
 extern unsigned long flashbase;
 void check_chip_supported(const struct flashchip *chip);
 int check_max_decode(enum chipbustype buses, uint32_t size);
-char *extract_programmer_param(const char *param_name);
+int extract_programmer_param(const char const *param_name, char **value);
+
 
 /* spi.c */
 enum spi_controller {
diff --git a/rayer_spi.c b/rayer_spi.c
index b312610..4fbf55a 100644
--- a/rayer_spi.c
+++ b/rayer_spi.c
@@ -103,8 +103,7 @@ int rayer_spi_init(void)
 	enum rayer_type rayer_type = TYPE_RAYER;
 
 	/* Non-default port requested? */
-	arg = extract_programmer_param("iobase");
-	if (arg) {
+	if (!extract_programmer_param("iobase", &arg)) {
 		char *endptr = NULL;
 		unsigned long tmp;
 		tmp = strtoul(arg, &endptr, 0);
@@ -127,17 +126,16 @@ int rayer_spi_init(void)
 			msg_pinfo("Non-default I/O base requested. This will "
 				  "not change the hardware settings.\n");
 		}
+		free(arg);
 	} else {
 		/* Pick a default value for the I/O base. */
 		lpt_iobase = 0x378;
 	}
-	free(arg);
-	
+
 	msg_pdbg("Using address 0x%x as I/O base for parallel port access.\n",
 		 lpt_iobase);
 
-	arg = extract_programmer_param("type");
-	if (arg) {
+	if (!extract_programmer_param("type", &arg)) {
 		if (!strcasecmp(arg, "rayer")) {
 			rayer_type = TYPE_RAYER;
 		} else if (!strcasecmp(arg, "xilinx")) {
@@ -147,8 +145,8 @@ int rayer_spi_init(void)
 			free(arg);
 			return 1;
 		}
+		free(arg);
 	}
-	free(arg);
 	switch (rayer_type) {
 	case TYPE_RAYER:
 		msg_pdbg("Using RayeR SPIPGM pinout.\n");
diff --git a/serprog.c b/serprog.c
index b179ea4..2ccc0ed 100644
--- a/serprog.c
+++ b/serprog.c
@@ -379,8 +379,7 @@ int serprog_init(void)
 	int have_device = 0;
 
 	/* the parameter is either of format "dev=/dev/device:baud" or "ip=ip:port" */
-	device = extract_programmer_param("dev");
-	if (device && strlen(device)) {
+	if (!extract_programmer_param("dev", &device)) {
 		baudport = strstr(device, ":");
 		if (baudport) {
 			/* Split device from baudrate. */
@@ -401,23 +400,20 @@ int serprog_init(void)
 			}
 			have_device++;
 		}
-	}
-	if (device && !strlen(device)) {
-		msg_perr("Error: No device specified.\n"
-			 "Use flashrom -p serprog:dev=/dev/device:baud\n");
-		free(device);
-		return 1;
-	}
-	free(device);
 
-	device = extract_programmer_param("ip");
-	if (have_device && device) {
-		msg_perr("Error: Both host and device specified.\n"
-			 "Please use either dev= or ip= but not both.\n");
 		free(device);
-		return 1;
 	}
-	if (device && strlen(device)) {
+
+
+	if (!extract_programmer_param("ip", &device)) {
+
+		if (have_device) {
+			msg_perr("Error: Both host and device specified.\n"
+					"Please use either dev= or ip= but not both.\n");
+			free(device);
+			return 1;
+		}
+
 		baudport = strstr(device, ":");
 		if (baudport) {
 			/* Split host from port. */
@@ -438,14 +434,9 @@ int serprog_init(void)
 			}
 			have_device++;
 		}
-	}
-	if (device && !strlen(device)) {
-		msg_perr("Error: No host specified.\n"
-			 "Use flashrom -p serprog:ip=ipaddr:port\n");
+
 		free(device);
-		return 1;
 	}
-	free(device);
 
 	if (!have_device) {
 		msg_perr("Error: Neither host nor device specified.\n"
@@ -535,8 +526,7 @@ int serprog_init(void)
 			spi_programmer_serprog.max_data_read = v;
 			msg_pdbg(MSGHEADER "Maximum read-n length is %d\n", v);
 		}
-		spispeed = extract_programmer_param("spispeed");
-		if (spispeed && strlen(spispeed)) {
+		if (!extract_programmer_param("spispeed", &spispeed)) {
 			uint32_t f_spi_req, f_spi;
 			uint8_t buf[4];
 			char *f_spi_suffix;
@@ -578,8 +568,9 @@ int serprog_init(void)
 					 "It was actually set to %u Hz\n", f_spi_req, f_spi);
 			} else
 				msg_pdbg(MSGHEADER "Setting SPI clock rate to %u Hz failed!\n", f_spi_req);
+
+			free(spispeed);
 		}
-		free(spispeed);
 		bt = serprog_buses_supported;
 		if (sp_docommand(S_CMD_S_BUSTYPE, 1, &bt, 0, NULL))
 			return 1;
-- 
1.7.12.3





More information about the flashrom mailing list