Patchwork [2/4] rbd: allow escaping in config string

login
register
mail settings
Submitter Sage Weil
Date Sept. 15, 2011, 9:11 p.m.
Message ID <1316121071-7690-3-git-send-email-sage@newdream.net>
Download mbox | patch
Permalink /patch/114836/
State New
Headers show

Comments

Sage Weil - Sept. 15, 2011, 9:11 p.m.
The config string is variously delimited by =, @, and /, depending on the
field.  Allow these characters to be escaped by preceeding them with \.

Signed-off-by: Sage Weil <sage@newdream.net>
---
 block/rbd.c |   29 +++++++++++++++++++++++++++--
 1 files changed, 27 insertions(+), 2 deletions(-)
Kevin Wolf - Sept. 19, 2011, 2:06 p.m.
Am 15.09.2011 23:11, schrieb Sage Weil:
> The config string is variously delimited by =, @, and /, depending on the
> field.  Allow these characters to be escaped by preceeding them with \.
> 
> Signed-off-by: Sage Weil <sage@newdream.net>
> ---
>  block/rbd.c |   29 +++++++++++++++++++++++++++--
>  1 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/block/rbd.c b/block/rbd.c
> index f64b2e0..43f0e63 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -104,8 +104,15 @@ static int qemu_rbd_next_tok(char *dst, int dst_len,
>      *p = NULL;
>  
>      if (delim != '\0') {
> -        end = strchr(src, delim);
> -        if (end) {
> +        for (end = src; *end; ++end) {
> +            if (*end == delim) {
> +                break;
> +            }
> +            if (*end == '\\') {
> +                end++;
> +            }
> +        }

If src ends with a backslash, you read beyond the end of the string.

> +        if (*end == delim) {
>              *p = end + 1;
>              *end = '\0';
>          }
> @@ -124,6 +131,19 @@ static int qemu_rbd_next_tok(char *dst, int dst_len,
>      return 0;
>  }
>  
> +static void qemu_rbd_unescape(char *src)
> +{
> +    char *p;
> +
> +    for (p = src; *src; ++src, ++p) {
> +        if (*src == '\\') {
> +            src++;
> +        }
> +        *p = *src;
> +    }
> +    *p = '\0';
> +}

This has the same problem.

Wouldn't it make sense to have the unescape integrated in
qemu_rbd_next_tok? Or are there any places where you would want to call
it without doing a qemu_rbd_unescape() afterwards?

Kevin
Sage Weil - Sept. 19, 2011, 8:33 p.m.
On Mon, 19 Sep 2011, Kevin Wolf wrote:
> If src ends with a backslash, you read beyond the end of the string.

Right, sending an updated patch.

> Wouldn't it make sense to have the unescape integrated in
> qemu_rbd_next_tok? Or are there any places where you would want to call
> it without doing a qemu_rbd_unescape() afterwards?

The conf string makes two passes through rbd_next_tok(), once to grab 
the whole conf string, and again to pull out each item.  We can't 
strip out escaping the first time through or else e.g. \: will turn into 
the : delimiter.

I thought about adding a flag to enable/disable the escaping, but 
explicitly doing the unescape seemed cleaner.

sage
>

Patch

diff --git a/block/rbd.c b/block/rbd.c
index f64b2e0..43f0e63 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -104,8 +104,15 @@  static int qemu_rbd_next_tok(char *dst, int dst_len,
     *p = NULL;
 
     if (delim != '\0') {
-        end = strchr(src, delim);
-        if (end) {
+        for (end = src; *end; ++end) {
+            if (*end == delim) {
+                break;
+            }
+            if (*end == '\\') {
+                end++;
+            }
+        }
+        if (*end == delim) {
             *p = end + 1;
             *end = '\0';
         }
@@ -124,6 +131,19 @@  static int qemu_rbd_next_tok(char *dst, int dst_len,
     return 0;
 }
 
+static void qemu_rbd_unescape(char *src)
+{
+    char *p;
+
+    for (p = src; *src; ++src, ++p) {
+        if (*src == '\\') {
+            src++;
+        }
+        *p = *src;
+    }
+    *p = '\0';
+}
+
 static int qemu_rbd_parsename(const char *filename,
                               char *pool, int pool_len,
                               char *snap, int snap_len,
@@ -148,6 +168,7 @@  static int qemu_rbd_parsename(const char *filename,
         ret = -EINVAL;
         goto done;
     }
+    qemu_rbd_unescape(pool);
 
     if (strchr(p, '@')) {
         ret = qemu_rbd_next_tok(name, name_len, p, '@', "object name", &p);
@@ -155,9 +176,11 @@  static int qemu_rbd_parsename(const char *filename,
             goto done;
         }
         ret = qemu_rbd_next_tok(snap, snap_len, p, ':', "snap name", &p);
+        qemu_rbd_unescape(snap);
     } else {
         ret = qemu_rbd_next_tok(name, name_len, p, ':', "object name", &p);
     }
+    qemu_rbd_unescape(name);
     if (ret < 0 || !p) {
         goto done;
     }
@@ -213,6 +236,7 @@  static int qemu_rbd_set_conf(rados_t cluster, const char *conf)
         if (ret < 0) {
             break;
         }
+        qemu_rbd_unescape(name);
 
         if (!p) {
             error_report("conf option %s has no value", name);
@@ -225,6 +249,7 @@  static int qemu_rbd_set_conf(rados_t cluster, const char *conf)
         if (ret < 0) {
             break;
         }
+        qemu_rbd_unescape(value);
 
         if (strcmp(name, "conf") == 0) {
             ret = rados_conf_read_file(cluster, value);