The existing test "if feature is already present" doesn't work for multiple features, and we are only using add_feature() for single feature additions anyway. Simplify the code by not allowing spaces in the feature string to be added. This way we can drop the complex "count new features" code. Moreover, print an error message if the existing features string is malformed. The old code calculated the width of the feature count twice, and the second time messed up the buffer length field passed to snprintf(); fix that, too. Finally, replace several strcat() invocations by one strncpy() call to make the code easier to review. Signed-off-by: Martin Wilck <mwilck@xxxxxxxx> --- libmultipath/structs.c | 64 +++++++++++++++++--------------------------------- 1 file changed, 21 insertions(+), 43 deletions(-) diff --git a/libmultipath/structs.c b/libmultipath/structs.c index 11e33676..828e7907 100644 --- a/libmultipath/structs.c +++ b/libmultipath/structs.c @@ -516,8 +516,7 @@ void setup_feature(struct multipath *mpp, char *feature) int add_feature(char **f, const char *n) { int c = 0, d, l; - char *e, *p, *t; - const char *q; + char *e, *t; if (!f) return 1; @@ -526,6 +525,11 @@ int add_feature(char **f, const char *n) if (!n || *n == '0') return 0; + if (strchr(n, ' ') != NULL) { + condlog(0, "internal error: feature \"%s\" contains spaces", n); + return 1; + } + /* default feature is null */ if(!*f) { @@ -543,55 +547,29 @@ int add_feature(char **f, const char *n) /* Get feature count */ c = strtoul(*f, &e, 10); - if (*f == e) - /* parse error */ + if (*f == e || (*e != ' ' && *e != '\0')) { + condlog(0, "parse error in feature string \"%s\"", *f); return 1; - - /* Check if we need to increase feature count space */ - l = strlen(*f) + strlen(n) + 1; - - /* Count new features */ - if ((c % 10) == 9) - l++; - c++; - q = n; - while (*q != '\0') { - if (*q == ' ' && q[1] != '\0' && q[1] != ' ') { - if ((c % 10) == 9) - l++; - c++; - } - q++; } + /* Add 1 digit and 1 space */ + l = strlen(e) + strlen(n) + 2; + + c++; + /* Check if we need more digits for feature count */ + for (d = c; d >= 10; d /= 10) + l++; + t = MALLOC(l + 1); if (!t) return 1; - memset(t, 0, l + 1); + /* e: old feature string with leading space, or "" */ + if (*e == ' ') + while (*(e + 1) == ' ') + e++; - /* Update feature count */ - d = c; - l = 1; - while (d > 9) { - d /= 10; - l++; - } - p = t; - snprintf(p, l + 2, "%0d ", c); - - /* Copy the feature string */ - p = strchr(*f, ' '); - - if (p) { - while (*p == ' ') - p++; - strcat(t, p); - strcat(t, " "); - } else { - p = t + strlen(t); - } - strcat(t, n); + snprintf(t, l + 1, "%0d%s %s", c, e, n); FREE(*f); *f = t; -- 2.14.0 -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel