Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752821AbbF2NjN (ORCPT ); Mon, 29 Jun 2015 09:39:13 -0400 Received: from mail-yk0-f178.google.com ([209.85.160.178]:36537 "EHLO mail-yk0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751754AbbF2NjC (ORCPT ); Mon, 29 Jun 2015 09:39:02 -0400 MIME-Version: 1.0 In-Reply-To: <5592487D.7080006@intel.com> References: <5592487D.7080006@intel.com> Date: Mon, 29 Jun 2015 16:39:01 +0300 Message-ID: Subject: Re: [PATCH] lib/bitmap.c: return -EINVAL for grouping errors in __bitmap_parselist From: Yury Norov To: Pan Xinhui Cc: "linux-kernel@vger.kernel.org" , Andrew Morton , Rasmus Villemoes , tj@kernel.org, peterz@infradead.org, sudeep.holla@arm.com, mina86@mina86.com, "mnipxh@163.com" , Alexey Klimov Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2405 Lines: 84 > Sometimes the input from user may cause an unexpected result. Could you please provide specific example? > > just like __bitmap_parse, we return -EINVAL if there is no avaiable digit in each > parsing procedures. > > Signed-off-by: Pan Xinhui Hello, Pan. (Adding Alexey Klimov, Rasmus Villemoes) > --- > lib/bitmap.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/lib/bitmap.c b/lib/bitmap.c > index 64c0926..995fca2 100644 > --- a/lib/bitmap.c > +++ b/lib/bitmap.c > @@ -504,7 +504,7 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen, > int nmaskbits) > { > unsigned a, b; > - int c, old_c, totaldigits; > + int c, old_c, totaldigits, ndigits; > const char __user __force *ubuf = (const char __user __force *)buf; > int exp_digit, in_range; > > @@ -514,6 +514,7 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen, > exp_digit = 1; > in_range = 0; > a = b = 0; > + ndigits = 0; > > /* Get the next cpu# or a range of cpu#'s */ > while (buflen) { > @@ -555,8 +556,10 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen, > if (!in_range) > a = b; > exp_digit = 0; > - totaldigits++; > + ndigits++; totaldigits++; I'm not happy with joining two statements to a single line. Maybe sometimes it's OK for loop iterators like while (a[i][j]) { i++; j++; } But here it looks nasty. Anyway, it's minor. > } > + if (ndigits == 0) > + return -EINVAL; You can avoid in-loop incrementation of ndigits if you'll save current totaldigits to ndigits before loop, and check ndigits against totaldigits after the loop: ndigits = totaldigits; while (...) { ... totaldigits++; } if (ndigits == totaldigits) return -EINVAL; Maybe it's a good point to rework initial __bitmap_parse() similar way... > if (!(a <= b)) > return -EINVAL; > if (b >= nmaskbits) > -- > 1.9.1 -- 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/