diff mbox

[v2,3/5] libmultipath: config parser: Allow '"' in strings

Message ID 20180307232620.15746-4-mwilck@suse.com (mailing list archive)
State Not Applicable, archived
Delegated to: christophe varoqui
Headers show

Commit Message

Martin Wilck March 7, 2018, 11:26 p.m. UTC
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(-)

Comments

Xose Vazquez Perez March 23, 2018, 4:59 p.m. UTC | #1
On 03/08/2018 12:26 AM, Martin Wilck wrote:

> 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"
> }


Could this work in the first place?

          product "2.5\" SSD"

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Martin Wilck March 23, 2018, 5:40 p.m. UTC | #2
On Fr, 2018-03-23 at 17:59 +0100, Xose Vazquez Perez wrote:
> On 03/08/2018 12:26 AM, Martin Wilck wrote:
> 
> > 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"
> > }
> 
> 
> Could this work in the first place?
> 
>           product "2.5\" SSD"

I'm not sure what you mean. For the current code, the answer is "no".
The parser doesn't treat '\' special in any way. For the parser, the
result of the above is the tuple ['product', '2.5\', 'SSD']. The config
file parser would discard the last tuple element (the cli command
parser takes more than 2 tuple elements, but not the config file
parser). The regexp parser would bail out on the lone backslash in
'2.5\'.

If you meant to say that, instead of using '""', I could have changed
the parser to use '\"' to represent a single double-quote character,
yes, that would have been possible, at the cost of a much bigger and
more error-prone rewrite of the parsing code.

Moreover, I think using '\"' has an addtional disadvantage compared to
'""'. As you know, the values for many options are regular expressions,
and the backslash has special meaning in regexps. Figuring out the
correct number of backslashes for complex regexps is hard anyway, and
if we give '\' special meaning in the option parser, too, we'd treat it
specially on two separate layers, making it even harder for users. The
double-quote character, OTOH, has no special meaning in regexps, so
that the two quoting mechanisms on the two layers are orthogonal.

Regards
Martin
diff mbox

Patch

diff --git a/libmultipath/parser.c b/libmultipath/parser.c
index 3d9656f47945..21151a16ad74 100644
--- a/libmultipath/parser.c
+++ b/libmultipath/parser.c
@@ -219,11 +219,13 @@  alloc_strvec(char *string)
 
 	in_string = 0;
 	while (1) {
+		int two_quotes = 0;
+
 		if (!vector_alloc_slot(strvec))
 			goto out;
 
 		start = cp;
-		if (*cp == '"') {
+		if (*cp == '"' && !(in_string && *(cp + 1) == '"')) {
 			cp++;
 			token = MALLOC(2);
 
@@ -246,11 +248,23 @@  alloc_strvec(char *string)
 			*(token + 1) = '\0';
 			cp++;
 		} else {
+
+		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;
+				}
+			}
+
 			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);