lkml.org 
[lkml]   [2015]   [Jun]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2015-06-30 04:01    [W:0.084 / U:0.488 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site