Nico Huber has uploaded this change for review. ( https://review.coreboot.org/25126
Change subject: Rework extract_programmer_param to remove exit call ......................................................................
Rework extract_programmer_param to remove exit call
- 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.
Change-Id: If26807f9a25d94b15358d298358a1cb8e951bb5d Signed-off-by: Niklas Söderlund niso@kth.se --- M buspirate_spi.c M chipset_enable.c M dediprog.c M dummyflasher.c M flash.h M flashrom.c M ft2232_spi.c M ichspi.c M internal.c M it87spi.c M linux_spi.c M ogp_spi.c M pcidev.c M pony_spi.c M programmer.h M rayer_spi.c M serprog.c 17 files changed, 190 insertions(+), 250 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/26/25126/1
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 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 @@ 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 @@ 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 @@ 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 @@
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 @@ 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 @@
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 @@ 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 @@ } }
- 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 @@ 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 @@ 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 @@ }
#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 @@ 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 4913536..4690c35 100644 --- a/flash.h +++ b/flash.h @@ -230,7 +230,6 @@ 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 @@ 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; + char *pair, *pairptr; + char *keyptr, *tmp;
- needlelen = strlen(needle); - if (!needlelen) { - msg_gerr("%s: empty needle! Please report a bug at " - "flashrom@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); + pair = strtok_r(haystack, ",", &pairptr); + while (pair) { + tmp = strtok_r(pair, "=", &keyptr); + if (!strcmp(tmp, needle)) { + tmp = strtok_r(NULL, "=", &keyptr);
- 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); + 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@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 @@ 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 @@ 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 @@ } 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 @@ 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 @@ 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"); + 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; - } else if (arg) { - msg_perr("Unknown argument for ich_spi_mode: %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"); + 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; - } else if (arg) { - msg_perr("Unknown argument for ich_spi_force: "%s" " - "(not "yes").\n", arg); - 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 @@ 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"); + 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; - } else if (arg) { - msg_perr("Unknown argument for boardenable: %s\n", arg); - 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"); + 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; - } else if (arg) { - msg_perr("Unknown argument for boardmismatch: %s\n", arg); - 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"); + 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; - } else if (arg) { - msg_perr("Unknown argument for laptop: %s\n", arg); - 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 @@ 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 @@ 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 @@ 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 7aee72b..d192a0e 100644 --- a/ogp_spi.c +++ b/ogp_spi.c @@ -109,13 +109,13 @@ { 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 c1b6d6a..0bff2d1 100644 --- a/pcidev.c +++ b/pcidev.c @@ -170,14 +170,14 @@ 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); exit(1); } + 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 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 @@ 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"); + 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); - } else if (arg){ - msg_perr("Error: Invalid programmer type specified.\n"); - 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 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 @@ 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 @@ 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 @@ 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 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 @@ } 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 @@ } 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 @@ 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 @@ "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;
-
Nico Huber (Code Review)