Messages in this thread Patch in this message | | | Date | Wed, 01 Jul 2015 09:37:05 +0800 | From | Pan Xinhui <> | Subject | Re: [PATCH] lib/bitmap.c: return -EINVAL for grouping errors in __bitmap_parselist |
| |
hi, Yury thanks for your nice reply.
On 2015年06月29日 21:39, Yury Norov wrote: >> Sometimes the input from user may cause an unexpected result. > > Could you please provide specific example? > I wrote some scripts to do some tests about irqs. echo "1-3," > /proc/irq/<xxx>/smp_affinity_list this command ends with ',' by mistake. actually __bitmap_parselist() will report "0-3" for the final result which is wrong.
>> >> just like __bitmap_parse, we return -EINVAL if there is no avaiable digit in each >> parsing procedures. >> >> Signed-off-by: Pan Xinhui <xinhuix.pan@intel.com> > > 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. >
thanks for pointing out my mistake about the code style :)
>> } >> + 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... >
your advice is a good idea, thanks. I am also thinking if we can rewrite them into one function for common codes.
thanks for your reply again :)
thanks xinhui
>> if (!(a <= b)) >> return -EINVAL; >> if (b >= nmaskbits) >> -- >> 1.9.1
| |