Fix parsing of ioengine strings
authorSteven Lang <tirea@google.com>
Wed, 16 Nov 2011 09:33:51 +0000 (10:33 +0100)
committerJens Axboe <axboe@kernel.dk>
Wed, 16 Nov 2011 09:33:51 +0000 (10:33 +0100)
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.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
parse.c

diff --git a/parse.c b/parse.c
index 9a65e31..8aaf799 100644 (file)
--- 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;
        }