| Message ID | 20190717071114.14772-3-richardw.yang@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | bitmap: refine bitmap_set | expand |
On Wed, Jul 17, 2019 at 03:11:14PM +0800, Wei Yang wrote: > Add a test for bitmap_set. There are three cases: > > * Both start and end is BITS_PER_LONG aligned > * Only start is BITS_PER_LONG aligned > * Only end is BITS_PER_LONG aligned > > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> Hi, Wei, Thanks for doing this. I've got a few comments below. > --- > tests/test-bitmap.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/tests/test-bitmap.c b/tests/test-bitmap.c > index cb7c5e462d..1f0123f604 100644 > --- a/tests/test-bitmap.c > +++ b/tests/test-bitmap.c > @@ -59,12 +59,45 @@ static void check_bitmap_copy_with_offset(void) > g_free(bmap3); > } > > +static void check_bitmap_set(void) > +{ > + unsigned long *bmap; > + > + bmap = bitmap_new(BMAP_SIZE); Need to free this. > + > + /* Both Aligned, set bits [BITS_PER_LONG, 2*BITS_PER_LONG] */ > + bitmap_set(bmap, BITS_PER_LONG, BITS_PER_LONG); > + g_assert_cmpint(find_first_bit(bmap, BITS_PER_LONG), ==, BITS_PER_LONG); Can check all 1's set correctly. g_assert_cmpuint(bmap[1], ==, -1ul); Can also make this at least across multiple long fields. > + g_assert_cmpint(find_next_zero_bit(bmap, 2 * BITS_PER_LONG, BITS_PER_LONG), > + ==, 2 * BITS_PER_LONG); > + > + bitmap_clear(bmap, 0, BMAP_SIZE); > + /* End Aligned, set bits [BITS_PER_LONG - 5, 2*BITS_PER_LONG] */ > + bitmap_set(bmap, BITS_PER_LONG - 5, BITS_PER_LONG + 5); Same here. > + g_assert_cmpint(find_first_bit(bmap, BITS_PER_LONG), > + ==, BITS_PER_LONG - 5); > + g_assert_cmpint(find_next_zero_bit(bmap, > + 2 * BITS_PER_LONG, BITS_PER_LONG - 5), > + ==, 2 * BITS_PER_LONG); > + > + bitmap_clear(bmap, 0, BMAP_SIZE); > + /* Start Aligned, set bits [BITS_PER_LONG, 2*BITS_PER_LONG + 5] */ > + bitmap_set(bmap, BITS_PER_LONG, BITS_PER_LONG + 5); And here. > + g_assert_cmpint(find_first_bit(bmap, BITS_PER_LONG), > + ==, BITS_PER_LONG); > + g_assert_cmpint(find_next_zero_bit(bmap, > + 2 * BITS_PER_LONG + 5, BITS_PER_LONG), > + ==, 2 * BITS_PER_LONG + 5); > +} > + > int main(int argc, char **argv) > { > g_test_init(&argc, &argv, NULL); > > g_test_add_func("/bitmap/bitmap_copy_with_offset", > check_bitmap_copy_with_offset); > + g_test_add_func("/bitmap/bitmap_set", > + check_bitmap_set); Can at least do the same test to bitmap_set_atomic too, simply by allowing your helper test function to take a func pointer: void (*bmap_set_func)(unsigned long *map, long i, long len); Then call with both bitmap_set{_atomic}. Thanks,
On Wed, Jul 17, 2019 at 03:43:11PM +0800, Peter Xu wrote: >On Wed, Jul 17, 2019 at 03:11:14PM +0800, Wei Yang wrote: >> Add a test for bitmap_set. There are three cases: >> >> * Both start and end is BITS_PER_LONG aligned >> * Only start is BITS_PER_LONG aligned >> * Only end is BITS_PER_LONG aligned >> >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > >Hi, Wei, > >Thanks for doing this. I've got a few comments below. > >> --- >> tests/test-bitmap.c | 33 +++++++++++++++++++++++++++++++++ >> 1 file changed, 33 insertions(+) >> >> diff --git a/tests/test-bitmap.c b/tests/test-bitmap.c >> index cb7c5e462d..1f0123f604 100644 >> --- a/tests/test-bitmap.c >> +++ b/tests/test-bitmap.c >> @@ -59,12 +59,45 @@ static void check_bitmap_copy_with_offset(void) >> g_free(bmap3); >> } >> >> +static void check_bitmap_set(void) >> +{ >> + unsigned long *bmap; >> + >> + bmap = bitmap_new(BMAP_SIZE); > >Need to free this. > oops, you are right. >> + >> + /* Both Aligned, set bits [BITS_PER_LONG, 2*BITS_PER_LONG] */ >> + bitmap_set(bmap, BITS_PER_LONG, BITS_PER_LONG); >> + g_assert_cmpint(find_first_bit(bmap, BITS_PER_LONG), ==, BITS_PER_LONG); > >Can check all 1's set correctly. > > g_assert_cmpuint(bmap[1], ==, -1ul); > >Can also make this at least across multiple long fields. > good suggestion >> + g_assert_cmpint(find_next_zero_bit(bmap, 2 * BITS_PER_LONG, BITS_PER_LONG), >> + ==, 2 * BITS_PER_LONG); >> + >> + bitmap_clear(bmap, 0, BMAP_SIZE); >> + /* End Aligned, set bits [BITS_PER_LONG - 5, 2*BITS_PER_LONG] */ >> + bitmap_set(bmap, BITS_PER_LONG - 5, BITS_PER_LONG + 5); > >Same here. > >> + g_assert_cmpint(find_first_bit(bmap, BITS_PER_LONG), >> + ==, BITS_PER_LONG - 5); >> + g_assert_cmpint(find_next_zero_bit(bmap, >> + 2 * BITS_PER_LONG, BITS_PER_LONG - 5), >> + ==, 2 * BITS_PER_LONG); >> + >> + bitmap_clear(bmap, 0, BMAP_SIZE); >> + /* Start Aligned, set bits [BITS_PER_LONG, 2*BITS_PER_LONG + 5] */ >> + bitmap_set(bmap, BITS_PER_LONG, BITS_PER_LONG + 5); > >And here. > >> + g_assert_cmpint(find_first_bit(bmap, BITS_PER_LONG), >> + ==, BITS_PER_LONG); >> + g_assert_cmpint(find_next_zero_bit(bmap, >> + 2 * BITS_PER_LONG + 5, BITS_PER_LONG), >> + ==, 2 * BITS_PER_LONG + 5); >> +} >> + >> int main(int argc, char **argv) >> { >> g_test_init(&argc, &argv, NULL); >> >> g_test_add_func("/bitmap/bitmap_copy_with_offset", >> check_bitmap_copy_with_offset); >> + g_test_add_func("/bitmap/bitmap_set", >> + check_bitmap_set); > >Can at least do the same test to bitmap_set_atomic too, simply by >allowing your helper test function to take a func pointer: > >void (*bmap_set_func)(unsigned long *map, long i, long len); > >Then call with both bitmap_set{_atomic}. > ok, let me take a look into this. >Thanks, > >-- >Peter Xu
diff --git a/tests/test-bitmap.c b/tests/test-bitmap.c index cb7c5e462d..1f0123f604 100644 --- a/tests/test-bitmap.c +++ b/tests/test-bitmap.c @@ -59,12 +59,45 @@ static void check_bitmap_copy_with_offset(void) g_free(bmap3); } +static void check_bitmap_set(void) +{ + unsigned long *bmap; + + bmap = bitmap_new(BMAP_SIZE); + + /* Both Aligned, set bits [BITS_PER_LONG, 2*BITS_PER_LONG] */ + bitmap_set(bmap, BITS_PER_LONG, BITS_PER_LONG); + g_assert_cmpint(find_first_bit(bmap, BITS_PER_LONG), ==, BITS_PER_LONG); + g_assert_cmpint(find_next_zero_bit(bmap, 2 * BITS_PER_LONG, BITS_PER_LONG), + ==, 2 * BITS_PER_LONG); + + bitmap_clear(bmap, 0, BMAP_SIZE); + /* End Aligned, set bits [BITS_PER_LONG - 5, 2*BITS_PER_LONG] */ + bitmap_set(bmap, BITS_PER_LONG - 5, BITS_PER_LONG + 5); + g_assert_cmpint(find_first_bit(bmap, BITS_PER_LONG), + ==, BITS_PER_LONG - 5); + g_assert_cmpint(find_next_zero_bit(bmap, + 2 * BITS_PER_LONG, BITS_PER_LONG - 5), + ==, 2 * BITS_PER_LONG); + + bitmap_clear(bmap, 0, BMAP_SIZE); + /* Start Aligned, set bits [BITS_PER_LONG, 2*BITS_PER_LONG + 5] */ + bitmap_set(bmap, BITS_PER_LONG, BITS_PER_LONG + 5); + g_assert_cmpint(find_first_bit(bmap, BITS_PER_LONG), + ==, BITS_PER_LONG); + g_assert_cmpint(find_next_zero_bit(bmap, + 2 * BITS_PER_LONG + 5, BITS_PER_LONG), + ==, 2 * BITS_PER_LONG + 5); +} + int main(int argc, char **argv) { g_test_init(&argc, &argv, NULL); g_test_add_func("/bitmap/bitmap_copy_with_offset", check_bitmap_copy_with_offset); + g_test_add_func("/bitmap/bitmap_set", + check_bitmap_set); g_test_run();
Add a test for bitmap_set. There are three cases: * Both start and end is BITS_PER_LONG aligned * Only start is BITS_PER_LONG aligned * Only end is BITS_PER_LONG aligned Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> --- tests/test-bitmap.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)