From 2e5180d38b33710506ab76dcf93f802864b9ec0f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Jan 2020 10:07:34 +0100 Subject: [PATCH 1/3] strv: get rid of strv_clear() Let's remove a function of questionnable utility. strv_clear() frees the items of a string array, but not the array itself. i.e. it half-drestructs a string array and makes it empty. This is not too useful an operation since we almost never need to just do that, we also want to free the whole thing. In fact, strv_clear() is only used in one of our .c file, and there it appears like unnecessary optimization, given that for each array with n elements it leaves the number of free()s we need to at O(n) which is not really an optimization at all (it goes from n+1 to n, that's all). Prompted by the discussions on #14605 --- src/basic/strv.c | 9 ++------- src/basic/strv.h | 2 -- src/libsystemd/sd-hwdb/hwdb-util.c | 6 +++--- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/basic/strv.c b/src/basic/strv.c index 74d20a9a95..096cb4e5d4 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -57,20 +57,15 @@ char *strv_find_startswith(char * const *l, const char *name) { return NULL; } -void strv_clear(char **l) { +char **strv_free(char **l) { char **k; if (!l) - return; + return NULL; for (k = l; *k; k++) free(*k); - *l = NULL; -} - -char **strv_free(char **l) { - strv_clear(l); return mfree(l); } diff --git a/src/basic/strv.h b/src/basic/strv.h index 85a49ab5c3..e7c2b1a604 100644 --- a/src/basic/strv.h +++ b/src/basic/strv.h @@ -25,8 +25,6 @@ char **strv_free_erase(char **l); DEFINE_TRIVIAL_CLEANUP_FUNC(char**, strv_free_erase); #define _cleanup_strv_free_erase_ _cleanup_(strv_free_erasep) -void strv_clear(char **l); - char **strv_copy(char * const *l); size_t strv_length(char * const *l) _pure_; diff --git a/src/libsystemd/sd-hwdb/hwdb-util.c b/src/libsystemd/sd-hwdb/hwdb-util.c index c83575c7c8..1a2da9c79b 100644 --- a/src/libsystemd/sd-hwdb/hwdb-util.c +++ b/src/libsystemd/sd-hwdb/hwdb-util.c @@ -543,7 +543,7 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr "Property expected, ignoring record with no properties"); r = -EINVAL; state = HW_NONE; - strv_clear(match_list); + match_list = strv_free(match_list); break; } @@ -571,7 +571,7 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr if (len == 0) { /* end of record */ state = HW_NONE; - strv_clear(match_list); + match_list = strv_free(match_list); break; } @@ -580,7 +580,7 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr "Property or empty line expected, got \"%s\", ignoring record", line); r = -EINVAL; state = HW_NONE; - strv_clear(match_list); + match_list = strv_free(match_list); break; } From 2aecc668878feebf8c88e2d38a657070a8ddb07a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Jan 2020 10:13:07 +0100 Subject: [PATCH 2/3] hwdb: use strv_extend() where we can --- src/libsystemd/sd-hwdb/hwdb-util.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/libsystemd/sd-hwdb/hwdb-util.c b/src/libsystemd/sd-hwdb/hwdb-util.c index 1a2da9c79b..658fa5fbcf 100644 --- a/src/libsystemd/sd-hwdb/hwdb-util.c +++ b/src/libsystemd/sd-hwdb/hwdb-util.c @@ -477,7 +477,6 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr _cleanup_fclose_ FILE *f = NULL; _cleanup_strv_free_ char **match_list = NULL; uint32_t line_number = 0; - char *match = NULL; int r = 0, err; f = fopen(filename, "re"); @@ -527,11 +526,7 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr /* start of record, first match */ state = HW_MATCH; - match = strdup(line); - if (!match) - return -ENOMEM; - - err = strv_consume(&match_list, match); + err = strv_extend(&match_list, line); if (err < 0) return err; @@ -549,11 +544,7 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr if (line[0] != ' ') { /* another match */ - match = strdup(line); - if (!match) - return -ENOMEM; - - err = strv_consume(&match_list, match); + err = strv_extend(&match_list, line); if (err < 0) return err; From d6bd2bb4441efc556cad2263a3051b1c3a6cebc6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Jan 2020 10:15:26 +0100 Subject: [PATCH 3/3] hwdb: fix error numbers passed to log_syntax() --- src/libsystemd/sd-hwdb/hwdb-util.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/libsystemd/sd-hwdb/hwdb-util.c b/src/libsystemd/sd-hwdb/hwdb-util.c index 658fa5fbcf..d790e8fd0b 100644 --- a/src/libsystemd/sd-hwdb/hwdb-util.c +++ b/src/libsystemd/sd-hwdb/hwdb-util.c @@ -446,7 +446,7 @@ static int insert_data(struct trie *trie, char **match_list, char *line, const c value = strchr(line, '='); if (!value) - return log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL, + return log_syntax(NULL, LOG_WARNING, filename, line_number, SYNTHETIC_ERRNO(EINVAL), "Key-value pair expected but got \"%s\", ignoring", line); value[0] = '\0'; @@ -457,7 +457,7 @@ static int insert_data(struct trie *trie, char **match_list, char *line, const c line++; if (isempty(line + 1) || isempty(value)) - return log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL, + return log_syntax(NULL, LOG_WARNING, filename, line_number, SYNTHETIC_ERRNO(EINVAL), "Empty %s in \"%s=%s\", ignoring", isempty(line + 1) ? "key" : "value", line, value); @@ -517,9 +517,8 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr break; if (line[0] == ' ') { - log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL, - "Match expected but got indented property \"%s\", ignoring line", line); - r = -EINVAL; + r = log_syntax(NULL, LOG_WARNING, filename, line_number, SYNTHETIC_ERRNO(EINVAL), + "Match expected but got indented property \"%s\", ignoring line", line); break; } @@ -534,9 +533,8 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr case HW_MATCH: if (len == 0) { - log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL, - "Property expected, ignoring record with no properties"); - r = -EINVAL; + r = log_syntax(NULL, LOG_WARNING, filename, line_number, SYNTHETIC_ERRNO(EINVAL), + "Property expected, ignoring record with no properties"); state = HW_NONE; match_list = strv_free(match_list); break; @@ -567,9 +565,8 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr } if (line[0] != ' ') { - log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL, - "Property or empty line expected, got \"%s\", ignoring record", line); - r = -EINVAL; + r = log_syntax(NULL, LOG_WARNING, filename, line_number, SYNTHETIC_ERRNO(EINVAL), + "Property or empty line expected, got \"%s\", ignoring record", line); state = HW_NONE; match_list = strv_free(match_list); break; @@ -583,7 +580,7 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr } if (state == HW_MATCH) - log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL, + log_syntax(NULL, LOG_WARNING, filename, line_number, 0, "Property expected, ignoring record with no properties"); return r;