Hi Andy, Thanks for thorough review.
On Wed, May 08, 2019 at 11:46:32AM +0300, Andy Shevchenko wrote: > On Tue, Apr 30, 2019 at 06:06:34PM -0700, Yury Norov wrote: > > bitmap_parse() is ineffective and full of opaque variables and opencoded > > parts. It leads to hard understanding and usage of it. This rework > > includes: > > - remove bitmap_shift_left() call from the cycle. Now it makes the > > complexity of the algorithm as O(nbits^2). In the suggested approach > > the input string is parsed in reverse direction, so no shifts needed; > > - relax requirement on a single comma and no white spaces between chunks. > > It is considered useful in scripting, and it aligns with > > bitmap_parselist(); > > - split bitmap_parse() to small readable helpers; > > - make an explicit calculation of the end of input line at the > > beginning, so users of the bitmap_parse() won't bother doing this. > > > +static inline bool in_str(const char *start, const char *ptr) > > +{ > > + return start <= ptr; > > +} > > + > > The explicit use of the conditional is better. > > -- > With Best Regards, > Andy Shevchenko I still think that is_str() is more verbose, but it's minor issue anyways, so I obey. Below is the patch that removes the function. It's up to Andrew finally, either apply it or not. Thanks, Yury >From 7438c15a0b165032a3e5a6d87daabe877dc8cbc8 Mon Sep 17 00:00:00 2001 From: Yury Norov <yno...@marvell.com> Date: Thu, 9 May 2019 17:54:23 -0700 Subject: [PATCH] lib: opencode in_str() Signed-off-by: Yury Norov <yno...@marvell.com> --- lib/bitmap.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/bitmap.c b/lib/bitmap.c index ebcf4700ebed..ecf93d2982a5 100644 --- a/lib/bitmap.c +++ b/lib/bitmap.c @@ -454,11 +454,6 @@ static inline bool end_of_region(char c) return __end_of_region(c) || end_of_str(c); } -static inline bool in_str(const char *start, const char *ptr) -{ - return start <= ptr; -} - /* * The format allows commas and whitespases at the beginning * of the region. @@ -473,7 +468,7 @@ static const char *bitmap_find_region(const char *str) static const char *bitmap_find_region_reverse(const char *start, const char *end) { - while (in_str(start, end) && __end_of_region(*end)) + while (start <= end && __end_of_region(*end)) end--; return end; @@ -618,7 +613,7 @@ static const char *bitmap_get_x32_reverse(const char *start, ret |= c << i; - if (!in_str(start, end) || __end_of_region(*end)) + if (start > end || __end_of_region(*end)) goto out; } @@ -653,7 +648,7 @@ int bitmap_parse(const char *start, unsigned int buflen, u32 *bitmap = (u32 *)maskp; int unset_bit; - while (in_str(start, (end = bitmap_find_region_reverse(start, end)))) { + while (start <= (end = bitmap_find_region_reverse(start, end))) { if (!chunks--) return -EOVERFLOW; -- 2.17.1