hi, all
        after a deep think, allocing a new buf is ugly.
new patch "lib/bitmap.c: fix some parsing issues and code style" is sent out.

any comments is welcome. :)

thanks
xinhui

On 2015年06月30日 18:02, Pan Xinhui wrote:

add __bitmap_parse_common to match any contents and return expected reslut.

as __bitmap_parse_common need NULL-terminated string, we alloc a new buf.

this patch also fix an unexpected parse result issue in __bitmap_parselist.

Signed-off-by: Pan Xinhui <xinhuix....@intel.com>
---
  lib/bitmap.c | 238 +++++++++++++++++++++++++++++++++--------------------------
  1 file changed, 134 insertions(+), 104 deletions(-)
---
change log
v3:
__bitmap_parselist now allow some extra input, like ",2, 3,,,,  5-8", at least 
one digit inside.
input like " " ", " is still not allowed.

V2:
__bitmap_parse_common need *base* to parse correct result

V1:
add __bitmap_parse_common and rewrite __bitmap_parse && __bitmap_parselist
---

diff --git a/lib/bitmap.c b/lib/bitmap.c
index 64c0926..f2095e1d 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -16,6 +16,8 @@
  #include <asm/page.h>
  #include <asm/uaccess.h>

+#include <linux/parser.h>
+#include <linux/slab.h>
  /*
   * bitmaps provide an array of bits, implemented using an an
   * array of unsigned longs.  The number of valid bits in a
@@ -331,6 +333,58 @@ again:
  EXPORT_SYMBOL(bitmap_find_next_zero_area_off);

  /*
+ * __bitmap_parse_common - parse expected number from buf
+ * Return 0 on success.
+ * there two patterns.
+ * if buf's contents did not match any of them, reutrn equivalent error.
+ * Notice buf's contents may be changed.
+ */
+static int __bitmap_parse_common(char *buf, unsigned int buflen,
+         unsigned long *a, unsigned long *b, int base)
+{
+    int ret;
+    int token;
+    const match_table_t table = {
+        {
+            .token = 1,
+            .pattern = "%x",
+        },
+        {
+            .token = 2,
+            .pattern = "%x-%x",
+        },
+        {
+            .token = 0,
+            .pattern = NULL,
+        }
+    };
+    substring_t substr[MAX_OPT_ARGS];
+
+    if (!buflen || !a)
+        return -EINVAL;
+
+    token = match_token((char *)buf, table, substr);
+    switch (token) {
+    case 1:
+        *substr[0].to = '\0';
+        ret = kstrtoul(substr[0].from, base, a);
+        if (b)
+            *b = *a;
+        break;
+    case 2:
+        *substr[0].to = '\0';
+        *substr[1].to = '\0';
+        ret = kstrtoul(substr[0].from, base, a);
+        ret |= b ? kstrtoul(substr[1].from, base, b) : -EINVAL;
+        break;
+    default:
+        ret = -EINVAL;
+        break;
+    }
+    return ret;
+}
+
+/*
   * Bitmap printing & parsing functions: first version by Nadia Yvette 
Chambers,
   * second version by Paul Jackson, third by Joe Korty.
   */
@@ -359,57 +413,45 @@ int __bitmap_parse(const char *buf, unsigned int buflen,
          int is_user, unsigned long *maskp,
          int nmaskbits)
  {
-    int c, old_c, totaldigits, ndigits, nchunks, nbits;
+    int nchunks, nbits, ret;
+    unsigned long a;
      u32 chunk;
      const char __user __force *ubuf = (const char __user __force *)buf;
+    char *kbuf, *endp;
+
+    if (!buflen)
+        return -EINVAL;
+    kbuf = kmalloc(buflen + 1, GFP_KERNEL);
+    if (!kbuf)
+        return -ENOMEM;
+    if (is_user) {
+        if (copy_from_user(kbuf, ubuf, buflen) != 0) {
+            kfree(kbuf);
+            return -EFAULT;
+        }
+    } else
+        memcpy(kbuf, buf, buflen);
+    kbuf[buflen] = '\0';
+    buf = strim(kbuf);

      bitmap_zero(maskp, nmaskbits);

-    nchunks = nbits = totaldigits = c = 0;
+    nchunks = nbits = 0;
      do {
-        chunk = ndigits = 0;
-
-        /* Get the next chunk of the bitmap */
-        while (buflen) {
-            old_c = c;
-            if (is_user) {
-                if (__get_user(c, ubuf++))
-                    return -EFAULT;
-            }
-            else
-                c = *buf++;
-            buflen--;
-            if (isspace(c))
-                continue;
-
-            /*
-             * If the last character was a space and the current
-             * character isn't '\0', we've got embedded whitespace.
-             * This is a no-no, so throw an error.
-             */
-            if (totaldigits && c && isspace(old_c))
-                return -EINVAL;
-
-            /* A '\0' or a ',' signal the end of the chunk */
-            if (c == '\0' || c == ',')
-                break;
-
-            if (!isxdigit(c))
-                return -EINVAL;
-
-            /*
-             * Make sure there are at least 4 free bits in 'chunk'.
-             * If not, this hexdigit will overflow 'chunk', so
-             * throw an error.
-             */
-            if (chunk & ~((1UL << (CHUNKSZ - 4)) - 1))
-                return -EOVERFLOW;
-
-            chunk = (chunk << 4) | hex_to_bin(c);
-            ndigits++; totaldigits++;
+        endp = strchr(buf, ',');
+        if (endp)
+            *endp = '\0';
+        ret = __bitmap_parse_common((char *)buf, strlen(buf),
+                                &a, NULL, 16);
+        if (ret)
+            break;
+        buf = endp + 1;
+
+        if (unlikely(a > U32_MAX)) {
+            ret = -ERANGE;
+            break;
          }
-        if (ndigits == 0)
-            return -EINVAL;
+        chunk = (u32)a;
          if (nchunks == 0 && chunk == 0)
              continue;

@@ -417,11 +459,13 @@ int __bitmap_parse(const char *buf, unsigned int buflen,
          *maskp |= chunk;
          nchunks++;
          nbits += (nchunks == 1) ? nbits_to_hold_value(chunk) : CHUNKSZ;
-        if (nbits > nmaskbits)
-            return -EOVERFLOW;
-    } while (buflen && c == ',');
-
-    return 0;
+        if (nbits > nmaskbits) {
+            ret = -EOVERFLOW;
+            break;
+        }
+    } while (endp);
+    kfree(kbuf);
+    return ret;
  }
  EXPORT_SYMBOL(__bitmap_parse);

@@ -503,70 +547,56 @@ static int __bitmap_parselist(const char *buf, unsigned 
int buflen,
          int is_user, unsigned long *maskp,
          int nmaskbits)
  {
-    unsigned a, b;
-    int c, old_c, totaldigits;
+    unsigned long a, b;
+    int ret = -EINVAL;
      const char __user __force *ubuf = (const char __user __force *)buf;
-    int exp_digit, in_range;
+    char *kbuf, *endp, *ibuf;
+
+    if (!buflen)
+        return -EINVAL;
+    ibuf = kbuf = kmalloc(buflen + 1, GFP_KERNEL);
+    if (!kbuf)
+        return -ENOMEM;
+    if (is_user) {
+        if (copy_from_user(kbuf, ubuf, buflen) != 0) {
+            kfree(kbuf);
+            return -EFAULT;
+        }
+    } else
+        memcpy(kbuf, buf, buflen);
+    kbuf[buflen] = '\0';

-    totaldigits = c = 0;
      bitmap_zero(maskp, nmaskbits);
      do {
-        exp_digit = 1;
-        in_range = 0;
-        a = b = 0;
-
-        /* Get the next cpu# or a range of cpu#'s */
-        while (buflen) {
-            old_c = c;
-            if (is_user) {
-                if (__get_user(c, ubuf++))
-                    return -EFAULT;
-            } else
-                c = *buf++;
-            buflen--;
-            if (isspace(c))
-                continue;
-
-            /*
-             * If the last character was a space and the current
-             * character isn't '\0', we've got embedded whitespace.
-             * This is a no-no, so throw an error.
-             */
-            if (totaldigits && c && isspace(old_c))
-                return -EINVAL;
-
-            /* A '\0' or a ',' signal the end of a cpu# or range */
-            if (c == '\0' || c == ',')
-                break;
-
-            if (c == '-') {
-                if (exp_digit || in_range)
-                    return -EINVAL;
-                b = 0;
-                in_range = 1;
-                exp_digit = 1;
-                continue;
-            }
-
-            if (!isdigit(c))
-                return -EINVAL;
-
-            b = b * 10 + (c - '0');
-            if (!in_range)
-                a = b;
-            exp_digit = 0;
-            totaldigits++;
+        endp = strchr(ibuf, ',');
+        if (endp)
+            *endp = '\0';
+        ibuf = strim(ibuf);
+        if (*ibuf == 0) {
+            ibuf = endp + 1;
+            continue;
+        }
+        ret = __bitmap_parse_common(ibuf, strlen(ibuf),
+                                &a, &b, 0);
+        if (ret)
+            break;
+        ibuf = endp + 1;
+
+        if (!(a <= b)) {
+            ret = -EINVAL;
+            break;
+        }
+        if (b >= nmaskbits) {
+            ret = -ERANGE;
+            break;
          }
-        if (!(a <= b))
-            return -EINVAL;
-        if (b >= nmaskbits)
-            return -ERANGE;
          while (a <= b) {
              set_bit(a, maskp);
              a++;
          }
-    } while (buflen && c == ',');
-    return 0;
+    } while (endp);
+    kfree(kbuf);
+    return ret;
  }

  int bitmap_parselist(const char *bp, unsigned long *maskp, int nmaskbits)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to