Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752662AbbGADUZ (ORCPT ); Tue, 30 Jun 2015 23:20:25 -0400 Received: from mga02.intel.com ([134.134.136.20]:1751 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752458AbbGADUS (ORCPT ); Tue, 30 Jun 2015 23:20:18 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,382,1432623600"; d="scan'208";a="516944376" Message-ID: <55935BDB.5070803@intel.com> Date: Wed, 01 Jul 2015 11:17:47 +0800 From: Pan Xinhui User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: "linux-kernel@vger.kernel.org" CC: Yury Norov , Andrew Morton , Rasmus Villemoes , tj@kernel.org, peterz@infradead.org, sudeep.holla@arm.com, mina86@mina86.com, "mnipxh@163.com" , Alexey Klimov Subject: Re: [PATCH V3] lib/bitmap.c: rewrite __bitmap_parse && __bitmap_parselist References: <5592487D.7080006@intel.com> <55934441.9010304@intel.com> <55924CD9.9020001@intel.com> <559253E0.2020602@intel.com> <55926929.4000901@intel.com> In-Reply-To: <55926929.4000901@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9889 Lines: 332 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 > --- > 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 > #include > > +#include > +#include > /* > * 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/