From: Luca Boccassi Date: Wed, 7 Apr 2021 21:58:12 +0000 (+0100) Subject: scsi_id: modernize and use extract_many_words instead of strsep X-Git-Tag: v249-rc1~446^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a9a49d2fea7a14a52ae2a93b93967fc51bcb5b5f;p=thirdparty%2Fsystemd.git scsi_id: modernize and use extract_many_words instead of strsep Also use standard error loggin/return pattern. Only cursory tested, by checking that with a simple config file the array is the same before/after. Not tested with actual scsi rules and devices, due to missing hardware. --- diff --git a/src/udev/scsi_id/scsi_id.c b/src/udev/scsi_id/scsi_id.c index 0c4a17c456d..3ac76072b09 100644 --- a/src/udev/scsi_id/scsi_id.c +++ b/src/udev/scsi_id/scsi_id.c @@ -19,9 +19,11 @@ #include "alloc-util.h" #include "build.h" #include "device-nodes.h" +#include "extract-word.h" #include "fd-util.h" #include "scsi_id.h" #include "string-util.h" +#include "strv.h" #include "strxcpyx.h" #include "udev-util.h" @@ -90,50 +92,6 @@ static void set_type(const char *from, char *to, size_t len) { strscpy(to, len, type); } -/* - * get_value: - * - * buf points to an '=' followed by a quoted string ("foo") or a string ending - * with a space or ','. - * - * Return a pointer to the NUL terminated string, returns NULL if no - * matches. - */ -static char *get_value(char **buffer) { - static const char *quote_string = "\"\n"; - static const char *comma_string = ",\n"; - char *val; - const char *end; - - if (**buffer == '"') { - /* - * skip leading quote, terminate when quote seen - */ - (*buffer)++; - end = quote_string; - } else - end = comma_string; - val = strsep(buffer, end); - if (val && end == quote_string) - /* - * skip trailing quote - */ - (*buffer)++; - - while (isspace(**buffer)) - (*buffer)++; - - return val; -} - -static int argc_count(char *opts) { - int i = 0; - while (*opts != '\0') - if (*opts++ == ' ') - i++; - return i; -} - /* * get_file_options: * @@ -145,14 +103,12 @@ static int argc_count(char *opts) { */ static int get_file_options(const char *vendor, const char *model, int *argc, char ***newargv) { - _cleanup_free_ char *buffer = NULL; + _cleanup_free_ char *buffer = NULL, + *vendor_in = NULL, *model_in = NULL, *options_in = NULL; /* read in from file */ + _cleanup_strv_free_ char **options_argv = NULL; _cleanup_fclose_ FILE *f; - char *buf; - char *str1; - char *vendor_in, *model_in, *options_in; /* read in from file */ - int lineno; - int c; - int retval = 0; + const char *buf; + int lineno, r; f = fopen(config_file, "re"); if (!f) { @@ -176,16 +132,16 @@ static int get_file_options(const char *vendor, const char *model, *newargv = NULL; lineno = 0; for (;;) { + _cleanup_free_ char *key = NULL, *value = NULL; + vendor_in = model_in = options_in = NULL; buf = fgets(buffer, MAX_BUFFER_LEN, f); if (!buf) break; lineno++; - if (buf[strlen(buffer) - 1] != '\n') { - log_error("Config file line %d too long", lineno); - break; - } + if (buf[strlen(buffer) - 1] != '\n') + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Config file line %d too long", lineno); while (isspace(*buf)) buf++; @@ -198,44 +154,36 @@ static int get_file_options(const char *vendor, const char *model, if (*buf == '#') continue; - str1 = strsep(&buf, "="); - if (str1 && strcaseeq(str1, "VENDOR")) { - str1 = get_value(&buf); - if (!str1) { - retval = log_oom(); - break; - } - vendor_in = str1; - - str1 = strsep(&buf, "="); - if (str1 && strcaseeq(str1, "MODEL")) { - str1 = get_value(&buf); - if (!str1) { - retval = log_oom(); - break; - } - model_in = str1; - str1 = strsep(&buf, "="); - } - } + r = extract_many_words(&buf, "=\",\n", 0, &key, &value, NULL); + if (r < 2) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Error parsing config file line %d '%s'", lineno, buffer); - if (str1 && strcaseeq(str1, "OPTIONS")) { - str1 = get_value(&buf); - if (!str1) { - retval = log_oom(); - break; + if (strcaseeq(key, "VENDOR")) { + vendor_in = TAKE_PTR(value); + + key = mfree(key); + r = extract_many_words(&buf, "=\",\n", 0, &key, &value, NULL); + if (r < 2) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Error parsing config file line %d '%s'", lineno, buffer); + + if (strcaseeq(key, "MODEL")) { + model_in = TAKE_PTR(value); + + key = mfree(key); + r = extract_many_words(&buf, "=\",\n", 0, &key, &value, NULL); + if (r < 2) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Error parsing config file line %d '%s'", lineno, buffer); } - options_in = str1; } + if (strcaseeq(key, "OPTIONS")) + options_in = TAKE_PTR(value); + /* * Only allow: [vendor=foo[,model=bar]]options=stuff */ - if (!options_in || (!vendor_in && model_in)) { - log_error("Error parsing config file line %d '%s'", lineno, buffer); - retval = -1; - break; - } + if (!options_in || (!vendor_in && model_in)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Error parsing config file line %d '%s'", lineno, buffer); if (!vendor) { if (!vendor_in) break; @@ -251,39 +199,30 @@ static int get_file_options(const char *vendor, const char *model, */ break; } - } - if (retval == 0) { - if (vendor_in != NULL || model_in != NULL || - options_in != NULL) { - /* - * Something matched. Allocate newargv, and store - * values found in options_in. - */ - strcpy(buffer, options_in); - c = argc_count(buffer) + 2; - *newargv = calloc(c, sizeof(**newargv)); - if (!*newargv) - retval = log_oom(); - else { - *argc = c; - c = 0; - /* - * argv[0] at 0 is skipped by getopt, but - * store the buffer address there for - * later freeing - */ - (*newargv)[c] = buffer; - for (c = 1; c < *argc; c++) - (*newargv)[c] = strsep(&buffer, " \t"); - buffer = NULL; - } - } else { - /* No matches */ - retval = 1; - } + vendor_in = mfree(vendor_in); + model_in = mfree(model_in); + options_in = mfree(options_in); + } - return retval; + + if (vendor_in == NULL && model_in == NULL && options_in == NULL) + return 1; /* No matches */ + + /* + * Something matched. Allocate newargv, and store + * values found in options_in. + */ + options_argv = strv_split(options_in, " \t"); + if (!options_argv) + return log_oom(); + r = strv_prepend(&options_argv, ""); /* getopt skips over argv[0] */ + if (r < 0) + return r; + *newargv = TAKE_PTR(options_argv); + *argc = strv_length(*newargv); + + return 0; } static void help(void) { @@ -391,9 +330,9 @@ static int set_options(int argc, char **argv, } static int per_dev_options(struct scsi_id_device *dev_scsi, int *good_bad, int *page_code) { + _cleanup_strv_free_ char **newargv = NULL; int retval; int newargc; - char **newargv = NULL; int option; *good_bad = all_good; @@ -436,10 +375,6 @@ static int per_dev_options(struct scsi_id_device *dev_scsi, int *good_bad, int * } } - if (newargv) { - free(newargv[0]); - free(newargv); - } return retval; } @@ -543,10 +478,10 @@ out: } int main(int argc, char **argv) { + _cleanup_strv_free_ char **newargv = NULL; int retval = 0; char maj_min_dev[MAX_PATH_LEN]; int newargc; - char **newargv = NULL; log_set_target(LOG_TARGET_AUTO); udev_parse_config(); @@ -585,10 +520,6 @@ int main(int argc, char **argv) { retval = scsi_id(maj_min_dev); exit: - if (newargv) { - free(newargv[0]); - free(newargv); - } log_close(); return retval; }