question on ext2fs_get_free_blocks() in e2fsprogs

In the course of going over Takashi's patches and general 32-bit 
overflow checking, I wound up with some questions on ext2fs_get_free_blocks.

As it is today, the behavior seems to be:

given a bitmap, look for "num" free blocks, starting from filesystem 
block "start" up to but -not- including filesystem block "finish"

if no map is provided, use f->block_map
if no start is provided, use the first block in the filesystem
if no finish is provided, also use the first block in the filesystem
   (i.e. search whole fs, I guess)
if no "num" is provided, search for only one block

If a suitable free block range can be found, return the first block of 
that range in *ret.

This all seems reasonably straightforward.

But what seems odd to me is:

     do {
         if (start + num - 1 > fs->super->s_blocks_count)
                 start = fs->super->s_first_data_block;
         if (ext2fs_fast_test_block_bitmap_range(map, start, num)) {
                 *ret = start;
                 return 0;
         }
         start++;
     } while (start != finish);

("b" in the code replaced with "start" here for clarity, not sure why 
"b" is in the code...)

1) is it really kosher to just move start back to the beginning of the 
filesystem, if the original request was for some other starting point?

2) the loop test seems odd; why (start != finish)? (Takashi had changed 
this to (start <= finish)(  But it seems to me that

   (start <= (finish - num + 1))

would be the better test; as it's written it will search for a starting 
block between (start) and (finish-1) inclusive, but callers seem to 
expect that the entire -range- of requested blocks will be between those 
boundaries, not just the starting block.

i.e. in ext2fs_allocate_group_table():
         last_blk = ext2fs_group_last_block(fs, group);
         ext2fs_get_free_blocks(fs, start_blk, last_blk, 1,
                                bmap, &new_blk);

and in new_table_block():
         last_block = ext2fs_group_last_block(fs, group);
         ext2fs_get_free_blocks(fs, first_block, last_block,
                                num, ctx->block_found_map, new_block);

surely the callers aren't interested in free ranges which extend past 
the group?

I don't knowif there's any practical bug here, but it's a bit 
bothersome, reading the code - unless I'm missing something.  :)

Thanks,

-Eric

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642