[PATCH 0/6] Fixes for config file parsing

[PATCH 0/6] Fixes for config file parsing

[Martin Wilck]

This series was motivated by the real-world problem that a user couldn't
figure out how to write a blacklist entry for a device called '1.8" SSD'.
Fixing this for good turned out to be a little tricky, therefore I also
added a test suite.

Martin Wilck (6):
  tests/uevent: fix -Wcast-qual errors
  tests: add unit tests for config file parser
  libmultipath: config parser: don't strip whitepace between quotes
  libmultipath: config parser: Allow '"' in strings
  libmultipath: config parser: fix corner case for double quotes
  multipath.conf(5): improve syntax documentation

 libmultipath/parser.c      |  65 ++++--
 multipath/multipath.conf.5 |  17 ++
 tests/Makefile             |   2 +-
 tests/globals.c            |   1 +
 tests/parser.c             | 497 +++++++++++++++++++++++++++++++++++++++++++++
 tests/uevent.c             |   8 +-
 6 files changed, 566 insertions(+), 24 deletions(-)
 create mode 100644 tests/parser.c

-- 
2.16.1

[PATCH 1/6] tests/uevent: fix -Wcast-qual errors

[Martin Wilck]

I overlooked these in my previous series of fixes.

Fixes: "libmultipath: fix compiler warnings for -Wcast-qual"
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 tests/uevent.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/uevent.c b/tests/uevent.c
index b7d6458710f4..0836b08d5b50 100644
--- a/tests/uevent.c
+++ b/tests/uevent.c
@@ -172,7 +172,7 @@ static void test_dm_name_good(void **state)
 	const char *name = uevent_get_dm_name(uev);
 
 	assert_string_equal(name, DM_NAME);
-	free((void*)name);
+	FREE_CONST(name);
 }
 
 static void test_dm_name_bad_0(void **state)
@@ -183,7 +183,7 @@ static void test_dm_name_bad_0(void **state)
 	uev->envp[3] = "DM_NAME" DM_NAME;
 	name = uevent_get_dm_name(uev);
 	assert_ptr_equal(name, NULL);
-	free((void*)name);
+	FREE_CONST(name);
 }
 
 static void test_dm_name_bad_1(void **state)
@@ -194,7 +194,7 @@ static void test_dm_name_bad_1(void **state)
 	uev->envp[3] = "DM_NAMES=" DM_NAME;
 	name = uevent_get_dm_name(uev);
 	assert_ptr_equal(name, NULL);
-	free((void*)name);
+	FREE_CONST(name);
 }
 
 static void test_dm_name_good_1(void **state)
@@ -206,7 +206,7 @@ static void test_dm_name_good_1(void **state)
 	uev->envp[2] = "DM_NAME=" DM_NAME;
 	name = uevent_get_dm_name(uev);
 	assert_string_equal(name, DM_NAME);
-	free((void*)name);
+	FREE_CONST(name);
 }
 
 static void test_dm_uuid_false_0(void **state)
-- 
2.16.1

[PATCH 2/6] tests: add unit tests for config file parser

[Martin Wilck]

Add test cases for parsing the config file.

Some of these tests currently fail unless the CPP macros at the top
are set to 1. The patches that follow fix them.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 tests/Makefile  |   2 +-
 tests/globals.c |   1 +
 tests/parser.c  | 497 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 499 insertions(+), 1 deletion(-)
 create mode 100644 tests/parser.c

diff --git a/tests/Makefile b/tests/Makefile
index f6b55836a434..231f73bc7332 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -3,7 +3,7 @@ include ../Makefile.inc
 CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathcmddir)
 LIBDEPS += -L$(multipathdir) -lmultipath -lcmocka
 
-TESTS := uevent
+TESTS := uevent parser
 
 .SILENT: $(TESTS:%=%.o)
 .PRECIOUS: $(TESTS:%=%-test)
diff --git a/tests/globals.c b/tests/globals.c
index 96a56515fd09..80f57bd3639a 100644
--- a/tests/globals.c
+++ b/tests/globals.c
@@ -6,6 +6,7 @@ struct udev *udev;
 int logsink = 0;
 struct config conf = {
 	.uid_attrs = "sd:ID_BOGUS",
+	.verbosity = 4,
 };
 
 struct config *get_multipath_config(void)
diff --git a/tests/parser.c b/tests/parser.c
new file mode 100644
index 000000000000..e77d7ef56caf
--- /dev/null
+++ b/tests/parser.c
@@ -0,0 +1,497 @@
+/*
+ * Copyright (c) 2018 SUSE Linux GmbH
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ *
+ */
+
+#include <stdbool.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <stdlib.h>
+#include <cmocka.h>
+// #include "list.h"
+#include "parser.h"
+#include "vector.h"
+
+#include "globals.c"
+
+/* Set these to 1 to get success for current broken behavior */
+/* Strip leading whitespace between quotes */
+#define LSTRIP_QUOTED_WSP 0
+/* Stop parsing at 2nd quote */
+#define TWO_QUOTES_ONLY 0
+
+#if TWO_QUOTES_ONLY
+static const char quote_marker[] = { '"', '\0' };
+#else
+static const char quote_marker[] = { '\0', '"', '\0' };
+#endif
+static bool is_quote(const char* token)
+{
+	return !memcmp(token, quote_marker, sizeof(quote_marker));
+}
+
+static char *test_file = "test.conf";
+
+/* Missing declaration */
+int validate_config_strvec(vector strvec, char *file);
+
+/* Stringify helpers */
+#define _str_(x) #x
+#define str(x) _str_(x)
+
+static int setup(void **state)
+{
+	return 0;
+}
+
+static int teardown(void **state)
+{
+	return 0;
+}
+
+static void test01(void **state)
+{
+	vector v = alloc_strvec("keyword value");
+	char *val;
+
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 2);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_string_equal(VECTOR_SLOT(v, 1), "value");
+
+	val = set_value(v);
+	assert_string_equal(val, "value");
+
+	free(val);
+	free_strvec(v);
+}
+
+static void test02(void **state)
+{
+	vector v = alloc_strvec("keyword \"value\"");
+	char *val;
+
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 4);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));;
+	assert_string_equal(VECTOR_SLOT(v, 2), "value");
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));;
+
+	val = set_value(v);
+	assert_string_equal(val, "value");
+
+	free(val);
+	free_strvec(v);
+}
+
+static void test03(void **state)
+{
+	vector v = alloc_strvec("keyword value\n");
+	char *val;
+
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 2);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_string_equal(VECTOR_SLOT(v, 1), "value");
+
+	val = set_value(v);
+	assert_string_equal(val, "value");
+
+	free(val);
+	free_strvec(v);
+}
+
+static void test04(void **state)
+{
+	vector v = alloc_strvec("keyword \t   value   \t \n   ");
+	char *val;
+
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 2);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_string_equal(VECTOR_SLOT(v, 1), "value");
+
+	val = set_value(v);
+	assert_string_equal(val, "value");
+
+	free(val);
+	free_strvec(v);
+}
+
+static void test05(void **state)
+{
+	vector v = alloc_strvec("keyword \t   value   \t ! comment  ");
+	char *val;
+
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 2);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_string_equal(VECTOR_SLOT(v, 1), "value");
+
+	val = set_value(v);
+	assert_string_equal(val, "value");
+
+	free(val);
+	free_strvec(v);
+}
+
+static void test06(void **state)
+{
+	vector v = alloc_strvec("keyword \t   value   # \n comment  ");
+	char *val;
+
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 2);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_string_equal(VECTOR_SLOT(v, 1), "value");
+
+	val = set_value(v);
+	assert_string_equal(val, "value");
+
+	free(val);
+	free_strvec(v);
+}
+
+static void test07(void **state)
+{
+	vector v = alloc_strvec("keyword \t   value   more  ");
+	char *val;
+
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 3);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_string_equal(VECTOR_SLOT(v, 1), "value");
+	assert_string_equal(VECTOR_SLOT(v, 2), "more");
+
+	val = set_value(v);
+	assert_string_equal(val, "value");
+
+	free(val);
+	free_strvec(v);
+}
+
+static void test08(void **state)
+{
+#define QUOTED08 "  value   more  "
+#define QUOTED08B "value   more  "
+	vector v = alloc_strvec("keyword \t \"" QUOTED08 "\"");
+	char *val;
+
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 4);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));;
+#if LSTRIP_QUOTED_WSP
+	assert_string_equal(VECTOR_SLOT(v, 2), QUOTED08B);
+#else
+	assert_string_equal(VECTOR_SLOT(v, 2), QUOTED08);
+#endif
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));;
+
+	val = set_value(v);
+#if LSTRIP_QUOTED_WSP
+	assert_string_equal(val, QUOTED08B);
+#else
+	assert_string_equal(val, QUOTED08);
+#endif
+	free(val);
+	free_strvec(v);
+}
+
+static void test09(void **state)
+{
+#define QUOTED09 "value # more"
+	vector v = alloc_strvec("keyword \"" QUOTED09 "\"");
+	char *val;
+
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 4);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));;
+	assert_string_equal(VECTOR_SLOT(v, 2), QUOTED09);
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));;
+
+	val = set_value(v);
+	assert_string_equal(val, QUOTED09);
+
+	free(val);
+	free_strvec(v);
+}
+
+static void test10(void **state)
+{
+#define QUOTED10 "value ! more"
+	vector v = alloc_strvec("keyword \"" QUOTED10 "\"");
+	char *val;
+
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 4);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));;
+	assert_string_equal(VECTOR_SLOT(v, 2), QUOTED10);
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));;
+
+	val = set_value(v);
+	assert_string_equal(val, QUOTED10);
+
+	free(val);
+	free_strvec(v);
+}
+
+static void test11(void **state)
+{
+#define QUOTED11 "value comment"
+	vector v = alloc_strvec("keyword\"" QUOTED11 "\"");
+	char *val;
+
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 4);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));;
+	assert_string_equal(VECTOR_SLOT(v, 2), QUOTED11);
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));;
+
+	val = set_value(v);
+	assert_string_equal(val, QUOTED11);
+
+	free(val);
+	free_strvec(v);
+}
+
+static void test12(void **state)
+{
+	vector v = alloc_strvec("key\"word\"");
+	char *val;
+
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 4);
+	assert_string_equal(VECTOR_SLOT(v, 0), "key");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));;
+	assert_string_equal(VECTOR_SLOT(v, 2), "word");
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));;
+
+	val = set_value(v);
+	assert_string_equal(val, "word");
+
+	free(val);
+	free_strvec(v);
+}
+
+static void test13(void **state)
+{
+	vector v = alloc_strvec("keyword value \"quoted\"");
+	char *val;
+
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 5);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_string_equal(VECTOR_SLOT(v, 1), "value");
+	assert_true(is_quote(VECTOR_SLOT(v, 2)));;
+	assert_string_equal(VECTOR_SLOT(v, 3), "quoted");
+	assert_true(is_quote(VECTOR_SLOT(v, 4)));;
+
+	val = set_value(v);
+	assert_string_equal(val, "value");
+
+	free(val);
+	free_strvec(v);
+}
+
+static void test14(void **state)
+{
+	vector v = alloc_strvec("keyword \"value \"  comment\"\"");
+	char *val;
+
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+#if  TWO_QUOTES_ONLY
+	assert_int_equal(VECTOR_SIZE(v), 7);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));;
+	assert_string_equal(VECTOR_SLOT(v, 2), "value ");
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));;
+	assert_string_equal(VECTOR_SLOT(v, 4), "comment");
+	assert_true(is_quote(VECTOR_SLOT(v, 5)));;
+	assert_true(is_quote(VECTOR_SLOT(v, 6)));;
+
+	val = set_value(v);
+	assert_string_equal(val, "value ");
+
+#else
+	assert_int_equal(VECTOR_SIZE(v), 5);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));;
+	assert_string_equal(VECTOR_SLOT(v, 2), "value ");
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));;
+	assert_string_equal(VECTOR_SLOT(v, 4), "  comment\"");
+
+	val = set_value(v);
+	assert_string_equal(val, "value ");
+
+#endif
+	free(val);
+	free_strvec(v);
+}
+
+static void test15(void **state)
+{
+#define QUOTED15 "word  value\n  comment"
+	vector v = alloc_strvec("key\"" QUOTED15 "\"");
+	char *val;
+
+	assert_int_equal(VECTOR_SIZE(v), 4);
+	assert_string_equal(VECTOR_SLOT(v, 0), "key");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));;
+	assert_string_equal(VECTOR_SLOT(v, 2), QUOTED15);
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));;
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+
+	val = set_value(v);
+	assert_string_equal(val, QUOTED15);
+
+	free(val);
+	free_strvec(v);
+}
+
+static void test16(void **state)
+{
+	vector v = alloc_strvec("keyword \"2.5\"\" SSD\"");
+	char *val;
+
+#if TWO_QUOTES_ONLY
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 6);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));;
+	assert_string_equal(VECTOR_SLOT(v, 2), "2.5");
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));;
+	assert_string_equal(VECTOR_SLOT(v, 4), "SSD");
+	assert_true(is_quote(VECTOR_SLOT(v, 5)));;
+
+	val = set_value(v);
+	assert_string_equal(val, "2.5");
+#else
+	assert_int_equal(VECTOR_SIZE(v), 4);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));;
+	assert_string_equal(VECTOR_SLOT(v, 2), "2.5\" SSD");
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));;
+
+	val = set_value(v);
+	assert_string_equal(val, "2.5\" SSD");
+#endif
+	free(val);
+	free_strvec(v);
+}
+
+static void test17(void **state)
+{
+	vector v = alloc_strvec("keyword \"\"\"\"\" is empty\"");
+ 	char *val;
+#if TWO_QUOTES_ONLY
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 6);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));;
+	assert_true(is_quote(VECTOR_SLOT(v, 2)));;
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));;
+#if LSTRIP_QUOTED_WSP
+	assert_string_equal(VECTOR_SLOT(v, 4), "is empty");
+#else
+	assert_string_equal(VECTOR_SLOT(v, 4), " is empty");
+#endif
+	assert_true(is_quote(VECTOR_SLOT(v, 5)));;
+
+	val = set_value(v);
+	assert_string_equal(val, "");
+#else
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 4);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));;
+	assert_string_equal(VECTOR_SLOT(v, 2), "\"\" is empty");
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));;
+
+	val = set_value(v);
+	assert_string_equal(val, "\"\" is empty");
+#endif
+	free(val);
+	free_strvec(v);
+}
+
+static void test18(void **state)
+{
+	vector v = alloc_strvec("keyword \"\"\"\"");
+ 	char *val;
+#if TWO_QUOTES_ONLY
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 5);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));;
+	assert_true(is_quote(VECTOR_SLOT(v, 2)));;
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));;
+	assert_true(is_quote(VECTOR_SLOT(v, 4)));;
+
+	val = set_value(v);
+	assert_string_equal(val, "");
+#else
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+	assert_int_equal(VECTOR_SIZE(v), 4);
+	assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));;
+	assert_string_equal(VECTOR_SLOT(v, 2), "\"");
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));;
+
+	val = set_value(v);
+	assert_string_equal(val, "\"");
+#endif
+	free(val);
+	free_strvec(v);
+}
+
+int test_config_parser(void)
+{
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test(test01),
+		cmocka_unit_test(test02),
+		cmocka_unit_test(test03),
+		cmocka_unit_test(test04),
+		cmocka_unit_test(test05),
+		cmocka_unit_test(test06),
+		cmocka_unit_test(test07),
+		cmocka_unit_test(test08),
+		cmocka_unit_test(test09),
+		cmocka_unit_test(test10),
+		cmocka_unit_test(test11),
+		cmocka_unit_test(test12),
+		cmocka_unit_test(test13),
+		cmocka_unit_test(test14),
+		cmocka_unit_test(test15),
+		cmocka_unit_test(test16),
+		cmocka_unit_test(test17),
+		cmocka_unit_test(test18),
+	};
+	return cmocka_run_group_tests(tests, setup, teardown);
+}
+
+int main(void)
+{
+	int ret = 0;
+
+	ret += test_config_parser();
+	return ret;
+}
-- 
2.16.1

[PATCH 3/6] libmultipath: config parser: don't strip whitepace between quotes

[Martin Wilck]

Between double quotes, the parser currently strips leading (but not
trailing) whitespace. That's inconsistent and unexpected. Fix it.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/parser.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libmultipath/parser.c b/libmultipath/parser.c
index 5caa2019a1a4..3d9656f47945 100644
--- a/libmultipath/parser.c
+++ b/libmultipath/parser.c
@@ -262,7 +262,8 @@ alloc_strvec(char *string)
 		}
 		vector_set_slot(strvec, token);
 
-		while ((isspace((int) *cp) || !isascii((int) *cp))
+		while ((!in_string &&
+			(isspace((int) *cp) || !isascii((int) *cp)))
 		       && *cp != '\0')
 			cp++;
 		if (*cp == '\0' || *cp == '!' || *cp == '#')
-- 
2.16.1

[PATCH 4/6] libmultipath: config parser: Allow '"' in strings

[Martin Wilck]

We have seen model strings lile '2.5" SSD' which can't be parsed
by the current config parser. This patch fixes this by allowing
'""' to represent a double quote character inside a a string.
The above model string could now be entered in the config file like this:

blacklist {
	  vendor SomeCorp
	  product "2.5"" SSD"
}

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/parser.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/libmultipath/parser.c b/libmultipath/parser.c
index 3d9656f47945..592269a9b5b1 100644
--- a/libmultipath/parser.c
+++ b/libmultipath/parser.c
@@ -223,7 +223,7 @@ alloc_strvec(char *string)
 			goto out;
 
 		start = cp;
-		if (*cp == '"') {
+		if (!in_string && *cp == '"') {
 			cp++;
 			token = MALLOC(2);
 
@@ -246,11 +246,25 @@ alloc_strvec(char *string)
 			*(token + 1) = '\0';
 			cp++;
 		} else {
+			int two_quotes = 0;
+
+		move_on:
 			while ((in_string ||
 				(!isspace((int) *cp) && isascii((int) *cp) &&
 				 *cp != '!' && *cp != '#' && *cp != '{' &&
 				 *cp != '}')) && *cp != '\0' && *cp != '"')
 				cp++;
+
+			/* Two consecutive double quotes - don't end string */
+			if (in_string && *cp == '"') {
+				if (*(cp + 1) == '"') {
+					two_quotes = 1;
+					cp += 2;
+					goto move_on;
+				} else
+					in_string = 0;
+			}
+
 			strlen = cp - start;
 			token = MALLOC(strlen + 1);
 
@@ -259,6 +273,16 @@ alloc_strvec(char *string)
 
 			memcpy(token, start, strlen);
 			*(token + strlen) = '\0';
+
+			/* Replace "" by " */
+			if (two_quotes) {
+				char *qq = strstr(token, "\"\"");
+				while (qq != NULL) {
+					memmove(qq + 1, qq + 2,
+						strlen + 1 - (qq + 2 - token));
+					qq = strstr(qq + 1, "\"\"");
+				}
+			}
 		}
 		vector_set_slot(strvec, token);
 
-- 
2.16.1

[PATCH 5/6] libmultipath: config parser: fix corner case for double quotes

[Martin Wilck]

A corner case of the previous patch are strings starting with a double quote,
such as '"prepended to itself is false" prepended to itself is false' or
'"" is the empty string', and in particular, the string '"' ("\"" in C
notation), which is indistinguishable from the "QUOTE" token in the parsed strvec.

This patch fixes that by introducing a special token that can't occur as part
of a normal string to indicate the beginning and end of a quoted string.

'"' is admittedly not a very likely keyword value for multipath.conf, but
a) this is a matter of correctness, b) we didn't think of '2.5"' before, either, and
c) the (*str != '"') expressions would need to be patched anyway to fix the
'string starting with "' case.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/parser.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/libmultipath/parser.c b/libmultipath/parser.c
index 592269a9b5b1..da8de305f680 100644
--- a/libmultipath/parser.c
+++ b/libmultipath/parser.c
@@ -186,6 +186,12 @@ snprint_keyword(char *buff, int len, char *fmt, struct keyword *kw,
 	return fwd;
 }
 
+static const char quote_marker[] = { '\0', '"', '\0' };
+static bool is_quote(const char* token)
+{
+	return !memcmp(token, quote_marker, sizeof(quote_marker));
+}
+
 vector
 alloc_strvec(char *string)
 {
@@ -225,17 +231,13 @@ alloc_strvec(char *string)
 		start = cp;
 		if (!in_string && *cp == '"') {
 			cp++;
-			token = MALLOC(2);
+			token = MALLOC(sizeof(quote_marker));
 
 			if (!token)
 				goto out;
 
-			*(token) = '"';
-			*(token + 1) = '\0';
-			if (in_string)
-				in_string = 0;
-			else
-				in_string = 1;
+			memcpy(token, quote_marker, sizeof(quote_marker));
+			in_string = 1;
 		} else if (!in_string && (*cp == '{' || *cp == '}')) {
 			token = MALLOC(2);
 
@@ -324,13 +326,13 @@ set_value(vector strvec)
 			(char *)VECTOR_SLOT(strvec, 0));
 		return NULL;
 	}
-	size = strlen(str);
-	if (size == 0) {
-		condlog(0, "option '%s' has empty value",
-			(char *)VECTOR_SLOT(strvec, 0));
-		return NULL;
-	}
-	if (*str != '"') {
+	if (!is_quote(str)) {
+		size = strlen(str);
+		if (size == 0) {
+			condlog(0, "option '%s' has empty value",
+				(char *)VECTOR_SLOT(strvec, 0));
+			return NULL;
+		}
 		alloc = MALLOC(sizeof (char) * (size + 1));
 		if (alloc)
 			memcpy(alloc, str, size);
@@ -354,7 +356,7 @@ set_value(vector strvec)
 				(char *)VECTOR_SLOT(strvec, 0));
 			return NULL;
 		}
-		if (*str == '"')
+		if (is_quote(str))
 			break;
 		tmp = alloc;
 		/* The first +1 is for the NULL byte. The rest are for the
@@ -460,7 +462,7 @@ validate_config_strvec(vector strvec, char *file)
 			(char *)VECTOR_SLOT(strvec, 0), line_nr, file);
 		return -1;
 	}
-	if (*str != '"') {
+	if (!is_quote(str)) {
 		if (VECTOR_SIZE(strvec) > 2)
 			condlog(0, "ignoring extra data starting with '%s' on line %d of %s", (char *)VECTOR_SLOT(strvec, 2), line_nr, file);
 		return 0;
@@ -472,7 +474,7 @@ validate_config_strvec(vector strvec, char *file)
 				line_nr, file);
 			return -1;
 		}
-		if (*str == '"') {
+		if (is_quote(str)) {
 			if (VECTOR_SIZE(strvec) > i + 1)
 				condlog(0, "ignoring extra data starting with '%s' on line %d of %s", (char *)VECTOR_SLOT(strvec, (i + 1)), line_nr, file);
 			return 0;
-- 
2.16.1

[PATCH 6/6] multipath.conf(5): improve syntax documentation

[Martin Wilck]

Describe the syntax of attribute / value pairs, comments, and quoted
strings, as well as the peculiarities of section beginnings and ends.
Also describe the newly added '""' feature.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipath/multipath.conf.5 | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index ab151e720d75..c4d0789475a3 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -67,6 +67,23 @@ recognized keywords for attributes or subsections depend on the
 section in which they occur.
 .LP
 .
+\fB<attribute>\fR and \fB<value>\fR must be on a single line.
+\fB<attribute>\fR is one of the keywords listed in this man page.
+\fB<value>\fR is either a simple word (containing no whitespace and none of the
+characters '\(dq', '#', and '!') or \fIone\fR string enclosed in double
+quotes ("..."). Outside a quoted string, text starting with '#', and '!' is
+regarded as a comment and ignored until the end of the line. Inside a quoted
+string, '#' and '!' are normal characters, and whitespace is preserved.
+To represent a double quote character inside a double quoted string, use two
+consecutive double quotes ('""'). Thus '2.5\(dq SSD' can be written as "2.5"" SSD".
+.LP
+.
+Opening braces ('{') must follow the (sub)section name on the same line. Closing
+braces ('}') that mark the end of a (sub)section must be the only non-whitespace
+character on the line. Whitespace is ignored except inside double quotes, thus
+the indentation shown in the above example is helpful for human readers but
+not mandatory.
+.LP
 .
 The following \fIsection\fP keywords are recognized:
 .TP 17
-- 
2.16.1

Re: [PATCH 0/6] Fixes for config file parsing

[Martin Wilck]

Just in case this is getting confusing, I've collected the latest
versions of my submissions, and what I gathered from others since
0.7.4, here (currently 83 commits ahead of last upstream commit):

https://github.com/openSUSE/multipath-tools/tree/upstream-queue

Please notify me if I missed something or made other mistakes.

Regards,
Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [PATCH 0/6] Fixes for config file parsing

[Martin Wilck]

On Tue, 2018-03-06 at 23:56 +0100, Martin Wilck wrote:
> This series was motivated by the real-world problem that a user
> couldn't
> figure out how to write a blacklist entry for a device called '1.8"
> SSD'.
> Fixing this for good turned out to be a little tricky, therefore I
> also
> added a test suite.

Christophe,

don't apply this yet. It breaks multipathd command parsing. Looking
into it.

Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel