input: handle escapes always in command parser

Previously, both the command parser and property expansion
(m_properties_expand_string) handled escapes with '\'. Move all escape
handling into the command parser, and remove it from the property code.

This removes the need to escape strings twice for commands that use
property expansion.

The command parser is practically rewritten: it uses m_option for the
actual parsing, and reduces hackish C-string handling.

bstr.h

@@ -152,7 +152,7 @@ static inline int bstr_find0(struct bstr haystack, const char *needle)
     return bstr_find(haystack, bstr0(needle));
 }
 
-static inline int bstr_eatstart0(struct bstr *s, char *prefix)
+static inline int bstr_eatstart0(struct bstr *s, const char *prefix)
 {
     return bstr_eatstart(s, bstr0(prefix));
 }

etc/input.conf

@@ -11,15 +11,13 @@
 #
 # Note that merely removing default key bindings from this file won't remove
 # the default bindings mplayer was compiled with, unless
-#   --input=nodefault-bindings
+#   --input=no-default-bindings
 # is specified.
 #
 # Lines starting with # are comments. Use SHARP to assign the # key.
 #
-# Some characters need to be escaped. In particular, if you want to display
+# Strings need to be quoted and escaped:
-# a '\' character as part of an osd_show_property_text OSD message, you have to
+# KEY  show_text "This is a single backslash: \\ and a quote: \" !"
-# escape 2 times:
-# key  osd_show_property_text "This is a single backslash: \\\\!"
 #
 # You can use modifier-key combinations like Shift+Left or Ctrl+Alt+x with
 # modifiers Shift, Ctrl, Alt and Meta, but note that currently reading

input/input.c

@@ -691,185 +691,217 @@ int mp_input_add_key_fd(struct input_ctx *ictx, int fd, int select,
     return 1;
 }
 
-static char *skip_ws(char *str)
+static bool read_token(bstr str, bstr *out_rest, bstr *out_token)
 {
-    while (str[0] == ' ' || str[0] == '\t')
+    bstr t = bstr_lstrip(str);
-        ++str;
+    int next = bstrcspn(t, WHITESPACE "#");
-    return str;
+    // Handle comments
+    if (t.start[next] == '#')
+        t = bstr_splice(t, 0, next);
+    if (!t.len)
+        return false;
+    *out_token = bstr_splice(t, 0, next);
+    *out_rest = bstr_cut(t, next);
+    return true;
 }
 
-static char *skip_no_ws(char *str)
+static bool eat_token(bstr *str, const char *tok)
 {
-    while (str[0] && !(str[0] == ' ' || str[0] == '\t'))
+    bstr rest, token;
-        ++str;
+    if (read_token(*str, &rest, &token) && bstrcmp0(token, tok) == 0) {
-    return str;
+        *str = rest;
+        return true;
+    }
+    return false;
 }
 
-mp_cmd_t *mp_input_parse_cmd(bstr str_b)
+static bool append_escape(bstr *code, char **str)
+{
+    if (code->len < 1)
+        return false;
+    char replace = 0;
+    switch (code->start[0]) {
+    case '"':  replace = '"';  break;
+    case '\\': replace = '\\'; break;
+    case 'b':  replace = '\b'; break;
+    case 'f':  replace = '\f'; break;
+    case 'n':  replace = '\n'; break;
+    case 'r':  replace = '\r'; break;
+    case 't':  replace = '\t'; break;
+    case 'e':  replace = '\x1b'; break;
+    case '\'': replace = '\''; break;
+    }
+    if (replace) {
+        *str = talloc_strndup_append_buffer(*str, &replace, 1);
+        *code = bstr_cut(*code, 1);
+        return true;
+    }
+    if (code->start[0] == 'x' && code->len >= 3) {
+        bstr num = bstr_splice(*code, 1, 3);
+        char c = bstrtoll(num, &num, 16);
+        if (!num.len)
+            return false;
+        *str = talloc_strndup_append_buffer(*str, &c, 1);
+        *code = bstr_cut(*code, 3);
+        return true;
+    }
+    if (code->start[0] == 'u' && code->len >= 5) {
+        bstr num = bstr_splice(*code, 1, 5);
+        int c = bstrtoll(num, &num, 16);
+        if (num.len)
+            return false;
+        *str = append_utf8_buffer(*str, c);
+        *code = bstr_cut(*code, 5);
+        return true;
+    }
+    return false;
+}
+
+static bool read_escaped_string(void *talloc_ctx, bstr *str, bstr *literal)
+{
+    bstr t = *str;
+    char *new = talloc_strdup(talloc_ctx, "");
+    while (t.len) {
+        if (t.start[0] == '"')
+            break;
+        if (t.start[0] == '\\') {
+            t = bstr_cut(t, 1);
+            if (!append_escape(&t, &new))
+                goto error;
+        } else {
+            new = talloc_strndup_append_buffer(new, t.start, 1);
+            t = bstr_cut(t, 1);
+        }
+    }
+    int len = str->len - t.len;
+    *literal = new ? bstr0(new) : bstr_splice(*str, 0, len);
+    *str = bstr_cut(*str, len);
+    return true;
+error:
+    talloc_free(new);
+    return false;
+}
+
+mp_cmd_t *mp_input_parse_cmd(bstr str)
 {
-    int i, l;
     int pausing = 0;
     int on_osd = MP_ON_OSD_AUTO;
-    char *ptr;
+    struct mp_cmd *cmd = NULL;
-    const mp_cmd_t *cmd_def;
-    mp_cmd_t *cmd = NULL;
     void *tmp = talloc_new(NULL);
-    char *str = bstrdup0(tmp, str_b);
 
-    str = skip_ws(str);
+    if (eat_token(&str, "pausing")) {
-
-    if (strncmp(str, "pausing ", 8) == 0) {
         pausing = 1;
-        str = &str[8];
+    } else if (eat_token(&str, "pausing_keep")) {
-    } else if (strncmp(str, "pausing_keep ", 13) == 0) {
         pausing = 2;
-        str = &str[13];
+    } else if (eat_token(&str, "pausing_toggle")) {
-    } else if (strncmp(str, "pausing_toggle ", 15) == 0) {
         pausing = 3;
-        str = &str[15];
+    } else if (eat_token(&str, "pausing_keep_force")) {
-    } else if (strncmp(str, "pausing_keep_force ", 19) == 0) {
         pausing = 4;
-        str = &str[19];
     }
 
-    str = skip_ws(str);
+    str = bstr_lstrip(str);
-
     for (const struct legacy_cmd *entry = legacy_cmds; entry->old; entry++) {
         size_t old_len = strlen(entry->old);
-        if (strncasecmp(entry->old, str, old_len) == 0) {
+        if (bstrcasecmp(bstr_splice(str, 0, old_len),
+                        (bstr) {(char *)entry->old, old_len}) == 0)
+        {
             mp_tmsg(MSGT_INPUT, MSGL_WARN, "Warning: command '%s' is "
                     "deprecated, replaced with '%s'. Fix your input.conf!\n",
                     entry->old, entry->new);
-            str = talloc_asprintf(tmp, "%s%s", entry->new, str + old_len);
+            bstr s = bstr_cut(str, old_len);
+            str = bstr0(talloc_asprintf(tmp, "%s%.*s", entry->new, BSTR_P(s)));
             break;
         }
     }
 
-    str = skip_ws(str);
+    if (eat_token(&str, "no-osd")) {
-
-    if (strncmp(str, "no-osd ", 7) == 0) {
         on_osd = MP_ON_OSD_NO;
-        str = &str[7];
     }
 
-    ptr = skip_no_ws(str);
+    int cmd_idx = 0;
-    if (*ptr != 0)
+    while (mp_cmds[cmd_idx].name != NULL) {
-        l = ptr - str;
+        if (eat_token(&str, mp_cmds[cmd_idx].name))
-    else
-        l = strlen(str);
-
-    if (l == 0)
-        goto error;
-
-    for (i = 0; mp_cmds[i].name != NULL; i++) {
-        const char *cmd = mp_cmds[i].name;
-        if (strncasecmp(cmd, str, l) == 0 && strlen(cmd) == l)
             break;
+        cmd_idx++;
     }
 
-    if (mp_cmds[i].name == NULL)
+    if (mp_cmds[cmd_idx].name == NULL)
         goto error;
 
-    cmd_def = &mp_cmds[i];
-
     cmd = talloc_ptrtype(NULL, cmd);
-    *cmd = (mp_cmd_t){
+    *cmd = mp_cmds[cmd_idx];
-        .id = cmd_def->id,
+    cmd->pausing = pausing;
-        .name = talloc_strdup(cmd, cmd_def->name),
+    cmd->on_osd = on_osd;
-        .pausing = pausing,
-        .on_osd = on_osd,
-    };
 
-    ptr = str;
+    for (int i = 0; i < MP_CMD_MAX_ARGS; i++) {
-
+        if (!cmd->args[i].type)
-    for (i = 0; ptr && i < MP_CMD_MAX_ARGS; i++) {
-        while (ptr[0] != ' ' && ptr[0] != '\t' && ptr[0] != '\0')
-            ptr++;
-        if (ptr[0] == '\0')
             break;
-        while (ptr[0] == ' ' || ptr[0] == '\t')
+        str = bstr_lstrip(str);
-            ptr++;
+        bstr arg = {0};
-        if (ptr[0] == '\0' || ptr[0] == '#')
+        if (cmd->args[i].type == MP_CMD_ARG_STRING &&
-            break;
+            bstr_eatstart0(&str, "\""))
-        cmd->args[i].type = cmd_def->args[i].type;
+        {
-        switch (cmd_def->args[i].type) {
+            if (!read_escaped_string(tmp, &str, &arg)) {
-        case MP_CMD_ARG_INT:
-            errno = 0;
-            cmd->args[i].v.i = atoi(ptr);
-            if (errno != 0) {
                 mp_tmsg(MSGT_INPUT, MSGL_ERR, "Command %s: argument %d "
-                        "isn't an integer.\n", cmd_def->name, i + 1);
+                        "has broken string escapes.\n", cmd->name, i + 1);
                 goto error;
             }
-            break;
+            if (!bstr_eatstart0(&str, "\"")) {
-        case MP_CMD_ARG_FLOAT:
+                mp_tmsg(MSGT_INPUT, MSGL_ERR, "Command %s: argument %d is "
-            errno = 0;
+                        "unterminated.\n", cmd->name, i + 1);
-            cmd->args[i].v.f = atof(ptr);
-            if (errno != 0) {
-                mp_tmsg(MSGT_INPUT, MSGL_ERR, "Command %s: argument %d "
-                        "isn't a float.\n", cmd_def->name, i + 1);
                 goto error;
             }
-            break;
+        } else {
-        case MP_CMD_ARG_STRING: {
+            if (!read_token(str, &str, &arg))
-            int term = ' ';
+                break;
-            if (*ptr == '\'' || *ptr == '"')
-                term = *ptr++;
-            char *argptr = talloc_size(cmd, strlen(ptr) + 1);
-            cmd->args[i].v.s = argptr;
-            while (1) {
-                if (*ptr == 0) {
-                    if (term == ' ')
-                        break;
-                    mp_tmsg(MSGT_INPUT, MSGL_ERR, "Command %s: argument %d is "
-                            "unterminated.\n", cmd_def->name, i + 1);
-                    goto error;
-                }
-                if (*ptr == term)
-                    break;
-                if (*ptr == '\\')
-                    ptr++;
-                if (*ptr != 0)
-                    *argptr++ = *ptr++;
-            }
-            *argptr = 0;
-            break;
         }
-        case 0:
+        // Prevent option API from trying to deallocate static strings
-            ptr = NULL;
+        cmd->args[i].v = ((struct mp_cmd_arg) {0}).v;
-            break;
+        struct m_option opt = {0};
-        default:
+        switch (cmd->args[i].type) {
-            mp_tmsg(MSGT_INPUT, MSGL_ERR, "Unknown argument %d\n", i);
+        case MP_CMD_ARG_INT:    opt.type = &m_option_type_int; break;
+        case MP_CMD_ARG_FLOAT:  opt.type = &m_option_type_float; break;
+        case MP_CMD_ARG_STRING: opt.type = &m_option_type_string; break;
+        default: abort();
         }
+        int r = m_option_parse(&opt, bstr0(cmd->name), arg, &cmd->args[i].v);
+        if (r < 0) {
+            mp_tmsg(MSGT_INPUT, MSGL_ERR, "Command %s: argument %d "
+                    "can't be parsed: %s.\n", cmd->name, i + 1,
+                    m_option_strerror(r));
+            goto error;
+        }
+        if (opt.type == &m_option_type_string)
+            cmd->args[i].v.s = talloc_steal(cmd, cmd->args[i].v.s);
+        cmd->nargs++;
     }
-    cmd->nargs = i;
 
-    int min_args;
+    bstr dummy;
-    for (min_args = 0; min_args < MP_CMD_MAX_ARGS
+    if (read_token(str, &dummy, &dummy)) {
-             && cmd_def->args[min_args].type
+        mp_tmsg(MSGT_INPUT, MSGL_ERR, "Command %s has trailing unused "
-             && !cmd_def->args[min_args].optional; min_args++);
+                "arguments: '%.*s'.\n", cmd->name, BSTR_P(str));
-    if (cmd->nargs < min_args) {
+        // Better make it fatal to make it clear something is wrong.
-        mp_tmsg(MSGT_INPUT, MSGL_ERR, "Command \"%s\" requires at least %d "
-                "arguments, we found only %d so far.\n", cmd_def->name,
-                min_args, cmd->nargs);
         goto error;
     }
 
-    for (; i < MP_CMD_MAX_ARGS && cmd_def->args[i].type; i++) {
+    int min_args = 0;
-        memcpy(&cmd->args[i], &cmd_def->args[i], sizeof(struct mp_cmd_arg));
+    while (min_args < MP_CMD_MAX_ARGS && cmd->args[min_args].type
-        if (cmd_def->args[i].type == MP_CMD_ARG_STRING
+           && !cmd->args[min_args].optional)
-            && cmd_def->args[i].v.s != NULL)
+    {
-            cmd->args[i].v.s = talloc_strdup(cmd, cmd_def->args[i].v.s);
+        min_args++;
+    }
+    if (cmd->nargs < min_args) {
+        mp_tmsg(MSGT_INPUT, MSGL_ERR, "Command %s requires at least %d "
+                "arguments, we found only %d so far.\n", cmd->name, min_args,
+                cmd->nargs);
+        goto error;
     }
-
-    if (i < MP_CMD_MAX_ARGS)
-        cmd->args[i].type = 0;
 
     talloc_free(tmp);
     return cmd;
 
- error:
+error:
-    mp_cmd_free(cmd);
+    talloc_free(cmd);
     talloc_free(tmp);
     return NULL;
 }

m_property.c

@@ -144,37 +144,11 @@ char *m_properties_expand_string(const m_option_t *prop_list, char *str,
                                  void *ctx)
 {
     int l, fr = 0, pos = 0, size = strlen(str) + 512;
-    char *p = NULL, *e, *ret = malloc(size), num_val;
+    char *p = NULL, *e, *ret = malloc(size);
     int skip = 0, lvl = 0, skip_lvl = 0;
 
     while (str[0]) {
-        if (str[0] == '\\') {
+        if (lvl > 0 && str[0] == ')') {
-            int sl = 1;
-            switch (str[1]) {
-            case 'e':
-                p = "\x1b", l = 1; break;
-            case 'n':
-                p = "\n", l = 1; break;
-            case 'r':
-                p = "\r", l = 1; break;
-            case 't':
-                p = "\t", l = 1; break;
-            case 'x':
-                if (str[2]) {
-                    char num[3] = { str[2], str[3], 0 };
-                    char *end = num;
-                    num_val = strtol(num, &end, 16);
-                    sl = end - num + 1;
-                    l = 1;
-                    p = &num_val;
-                } else
-                    l = 0;
-                break;
-            default:
-                p = str + 1, l = 1;
-            }
-            str += 1 + sl;
-        } else if (lvl > 0 && str[0] == ')') {
             if (skip && lvl <= skip_lvl)
                 skip = 0;
             lvl--, str++, l = 0;