All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin Wilck <mwilck@suse.com>
To: Christophe Varoqui <christophe.varoqui@opensvc.com>,
	Hannes Reinecke <hare@suse.de>,
	Benjamin Marzinski <bmarzins@redhat.com>
Cc: dm-devel@redhat.com
Subject: [PATCH 3/4] libmultipath: add_feature: allow only 1 feature
Date: Tue, 29 Aug 2017 00:05:35 +0200	[thread overview]
Message-ID: <20170828220536.13208-3-mwilck@suse.com> (raw)
In-Reply-To: <20170828220536.13208-1-mwilck@suse.com>

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@suse.com>
---
 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

  parent reply	other threads:[~2017-08-28 22:05 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-28 22:05 [PATCH 1/4] multipathd: don't flood system with sd_notify calls Martin Wilck
2017-08-28 22:05 ` [PATCH 2/4] libmultipath: add_feature: skip pointless NULL check Martin Wilck
2017-08-29  6:37   ` Hannes Reinecke
2017-08-29  7:21     ` Martin Wilck
2017-09-07 21:33   ` Benjamin Marzinski
2017-08-28 22:05 ` Martin Wilck [this message]
2017-08-29  6:39   ` [PATCH 3/4] libmultipath: add_feature: allow only 1 feature Hannes Reinecke
2017-09-07 21:42   ` Benjamin Marzinski
2017-09-07 22:37     ` Benjamin Marzinski
2017-08-28 22:05 ` [PATCH 4/4] multipath: delegate dangerous commands to multipathd Martin Wilck
2017-08-29  6:40   ` Hannes Reinecke
2017-09-07 21:57   ` Benjamin Marzinski
2017-09-08  8:16     ` Martin Wilck
2017-08-29  6:36 ` [PATCH 1/4] multipathd: don't flood system with sd_notify calls Hannes Reinecke
2017-09-07 21:31 ` Benjamin Marzinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170828220536.13208-3-mwilck@suse.com \
    --to=mwilck@suse.com \
    --cc=bmarzins@redhat.com \
    --cc=christophe.varoqui@opensvc.com \
    --cc=dm-devel@redhat.com \
    --cc=hare@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.