[PATCH] Fix parsing of ioengine strings

[PATCH] Fix parsing of ioengine strings

[Steven Lang]

This cleans up parsing of FIO_OPT_STR_STORE options which have defined
values (IE ioengine). �I made a few assumptions here...

1. If FIO_OPT_STR_STORE was used, that means a copy of the string is
desired, so it is always copied if the offset is non-zero.
2. If the values were the only allowed values, then FIO_OPT_STR would
have been used; therefore it is not an error to give a value not in
the list.
3. If an option callback is defined, then any values are ignored and
the callback is called to parse the string. �(Don't think this
currently applies, anywhere, but there is precendent in other options
skipping normal parsing if a callback is used.)
4. If no offsets are defined, and no callback is given, the behavior
is undefined - though this patch handles it cleanly and still calls
value callbacks; though I believe fio will still complain about a bad
options structure.

This fixes two cases (Restores it to previous behavior without
breaking new functionality).
1. External ioengines (The patch to a .so is never in the list of
possible values, so was never matching)
2. IO engines compiled in but not in the list in options.c

This patch should be applied to the 1.5x stable branch as well, though
it doesn't apply cleanly due to one line difference from my last
options patch.

diff --git a/parse.c b/parse.c
index 9a65e31..8aaf799 100644
--- a/parse.c
+++ b/parse.c
@@ -501,45 +501,54 @@ static int __handle_option(struct fio_option *o,
const char *ptr, void *data,
 	case FIO_OPT_STR_STORE: {
 		fio_opt_str_fn *fn = o->cb;

-		posval_sort(o, posval);
-
-		if ((o->roff1 || o->off1) && !o->posval[0].ival) {
-			vp = NULL;
-			goto match;
+		if (o->roff1 || o->off1) {
+			if (o->roff1)
+				cp = (char **) o->roff1;
+			else if (o->off1)
+				cp = td_var(data, o->off1);
+
+			*cp = strdup(ptr);
+		} else {
+			cp = NULL;
 		}

-		ret = 1;
-		for (i = 0; i < PARSE_MAX_VP; i++) {
-			vp = &posval[i];
-			if (!vp->ival || vp->ival[0] == '\0')
-				continue;
-			all_skipped = 0;
-			if (!strncmp(vp->ival, ptr, opt_len(ptr))) {
-				char *rest;
+		if (fn)
+			ret = fn(data, ptr);
+		else if (o->posval[0].ival) {
+			posval_sort(o, posval);

-				ret = 0;
-				if (vp->cb)
-					fn = vp->cb;
-match:
-				if (o->roff1)
-					cp = (char **) o->roff1;
-				else
-					cp = td_var(data, o->off1);
-				*cp = strdup(ptr);
-				rest = strstr(*cp, ":");
-				if (rest) {
-					*rest = '\0';
-					ptr = rest + 1;
-				} else if (vp && vp->cb)
-					ptr = NULL;
-				break;
+			ret = 1;
+			for (i = 0; i < PARSE_MAX_VP; i++) {
+				vp = &posval[i];
+				if (!vp->ival || vp->ival[0] == '\0')
+					continue;
+				all_skipped = 0;
+				if (!strncmp(vp->ival, ptr, opt_len(ptr))) {
+					char *rest;
+
+					ret = 0;
+					if (vp->cb)
+						fn = vp->cb;
+					rest = strstr(*cp ?: ptr, ":");
+					if (rest) {
+						if (*cp)
+							*rest = '\0';
+						ptr = rest + 1;
+					} else
+						ptr = NULL;
+					break;
+				}
 			}
 		}

-		if (ret && !all_skipped)
-			show_option_values(o);
-		else if (fn && ptr)
-			ret = fn(data, ptr);
+		if (!all_skipped) {
+			if (ret && !*cp)
+				show_option_values(o);
+			else if (ret && *cp)
+				ret = 0;
+			else if (fn && ptr)
+				ret = fn(data, ptr);
+		}

 		break;
 	}

Re: [PATCH] Fix parsing of ioengine strings

[Jens Axboe]

On 2011-11-15 02:41, Steven Lang wrote:
> This cleans up parsing of FIO_OPT_STR_STORE options which have defined
> values (IE ioengine).  I made a few assumptions here...
> 
> 1. If FIO_OPT_STR_STORE was used, that means a copy of the string is
> desired, so it is always copied if the offset is non-zero.
> 2. If the values were the only allowed values, then FIO_OPT_STR would
> have been used; therefore it is not an error to give a value not in
> the list.
> 3. If an option callback is defined, then any values are ignored and
> the callback is called to parse the string.  (Don't think this
> currently applies, anywhere, but there is precendent in other options
> skipping normal parsing if a callback is used.)
> 4. If no offsets are defined, and no callback is given, the behavior
> is undefined - though this patch handles it cleanly and still calls
> value callbacks; though I believe fio will still complain about a bad
> options structure.
> 
> This fixes two cases (Restores it to previous behavior without
> breaking new functionality).
> 1. External ioengines (The patch to a .so is never in the list of
> possible values, so was never matching)
> 2. IO engines compiled in but not in the list in options.c
> 
> This patch should be applied to the 1.5x stable branch as well, though
> it doesn't apply cleanly due to one line difference from my last
> options patch.

Applied (to both branches), thanks.

-- 
Jens Axboe