On Thu, Jul 21, 2011 at 5:46 AM, Eric Sandeen <sandeen@xxxxxxxxxx> wrote:
> Code to count the number of blocks in the last partial
> group is cut and pasted around the e2fsprogs codebase, and
> is wrong in at least one instancem as pointed out by
> Yongqiang Yang (but not fixed in this patch).
>
> Making this a helper function should improve matters.
>
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
>
> (I have not fixed up the spot Yongqiang Yang discovered,
> but this helper should now make that fix easy to do.)
>
> e2fsck/pass5.c | 6 +-----
> lib/ext2fs/alloc_sb.c | 10 +---------
> lib/ext2fs/blknum.c | 19 +++++++++++++++++++
> lib/ext2fs/closefs.c | 9 +--------
> lib/ext2fs/ext2fs.h | 1 +
> resize/online.c | 7 +------
> resize/resize2fs.c | 14 ++------------
> 7 files changed, 26 insertions(+), 40 deletions(-)
>
>
> diff --git a/e2fsck/pass5.c b/e2fsck/pass5.c
> index b423d28..73db4e8 100644
> --- a/e2fsck/pass5.c
> +++ b/e2fsck/pass5.c
> @@ -218,11 +218,7 @@ redo_counts:
> fs->super->s_reserved_gdt_blocks;
>
> count = 0;
> - cmp_block = fs->super->s_blocks_per_group;
> - if (group == (int)fs->group_desc_count - 1)
> - cmp_block =
> - ext2fs_blocks_count(fs->super) %
> - fs->super->s_blocks_per_group;
> + cmp_block = ext2fs_group_blocks_count(fs, group);
> }
Hi Eric,
You fixed the bug I pointed out here. but your commit log said it is not fixed.
Yongqiang.
>
> bitmap = 0;
> diff --git a/lib/ext2fs/alloc_sb.c b/lib/ext2fs/alloc_sb.c
> index d5fca3b..98aec52 100644
> --- a/lib/ext2fs/alloc_sb.c
> +++ b/lib/ext2fs/alloc_sb.c
> @@ -71,15 +71,7 @@ int ext2fs_reserve_super_and_bgd(ext2_filsys fs,
> if (new_desc_blk)
> ext2fs_mark_block_bitmap2(bmap, new_desc_blk);
>
> - if (group == fs->group_desc_count-1) {
> - num_blocks = (ext2fs_blocks_count(fs->super) -
> - fs->super->s_first_data_block) %
> - fs->super->s_blocks_per_group;
> - if (!num_blocks)
> - num_blocks = fs->super->s_blocks_per_group;
> - } else
> - num_blocks = fs->super->s_blocks_per_group;
> -
> + num_blocks = ext2fs_group_blocks_count(fs, group);
> num_blocks -= 2 + fs->inode_blocks_per_group + used_blks;
>
> return num_blocks ;
> diff --git a/lib/ext2fs/blknum.c b/lib/ext2fs/blknum.c
> index b3e6dca..f1bbe9b 100644
> --- a/lib/ext2fs/blknum.c
> +++ b/lib/ext2fs/blknum.c
> @@ -43,6 +43,25 @@ blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group)
> }
>
> /*
> + * Return the number of blocks in a group
> + */
> +int ext2fs_group_blocks_count(ext2_filsys fs, dgrp_t group)
> +{
> + int num_blocks;
> +
> + if (group == fs->group_desc_count-1) {
> + num_blocks = (ext2fs_blocks_count(fs->super) -
> + fs->super->s_first_data_block) %
> + fs->super->s_blocks_per_group;
> + if (!num_blocks)
> + num_blocks = fs->super->s_blocks_per_group;
> + } else
> + num_blocks = fs->super->s_blocks_per_group;
> +
> + return num_blocks;
> +}
> +
> +/*
> * Return the inode data block count
> */
> blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs,
> diff --git a/lib/ext2fs/closefs.c b/lib/ext2fs/closefs.c
> index 7a23e46..b13a0c9 100644
> --- a/lib/ext2fs/closefs.c
> +++ b/lib/ext2fs/closefs.c
> @@ -150,14 +150,7 @@ int ext2fs_super_and_bgd_loc(ext2_filsys fs,
> &ret_new_desc_blk2,
> &ret_used_blks);
>
> - if (group == fs->group_desc_count-1) {
> - numblocks = (ext2fs_blocks_count(fs->super) -
> - (blk64_t) fs->super->s_first_data_block) %
> - (blk64_t) fs->super->s_blocks_per_group;
> - if (!numblocks)
> - numblocks = fs->super->s_blocks_per_group;
> - } else
> - numblocks = fs->super->s_blocks_per_group;
> + numblocks = ext2fs_group_blocks_count(fs, group);
>
> if (ret_super_blk)
> *ret_super_blk = (blk_t)ret_super_blk2;
> diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
> index d3eb31d..5983adc 100644
> --- a/lib/ext2fs/ext2fs.h
> +++ b/lib/ext2fs/ext2fs.h
> @@ -731,6 +731,7 @@ extern errcode_t ext2fs_get_block_bitmap_range2(ext2fs_block_bitmap bmap,
> extern dgrp_t ext2fs_group_of_blk2(ext2_filsys fs, blk64_t);
> extern blk64_t ext2fs_group_first_block2(ext2_filsys fs, dgrp_t group);
> extern blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group);
> +extern int ext2fs_group_blocks_count(ext2_filsys fs, dgrp_t group);
> extern blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs,
> struct ext2_inode *inode);
> extern blk64_t ext2fs_inode_i_blocks(ext2_filsys fs,
> diff --git a/resize/online.c b/resize/online.c
> index 1d8d4ec..c2b98c6 100644
> --- a/resize/online.c
> +++ b/resize/online.c
> @@ -135,12 +135,7 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt,
> input.block_bitmap = ext2fs_block_bitmap_loc(new_fs, i);
> input.inode_bitmap = ext2fs_inode_bitmap_loc(new_fs, i);
> input.inode_table = ext2fs_inode_table_loc(new_fs, i);
> - input.blocks_count = sb->s_blocks_per_group;
> - if (i == new_fs->group_desc_count-1) {
> - input.blocks_count = ext2fs_blocks_count(new_fs->super) -
> - sb->s_first_data_block -
> - (i * sb->s_blocks_per_group);
> - }
> + input.blocks_count = ext2fs_group_blocks_count(new_fs, i);
> input.reserved_blocks = (blk_t) (percent * input.blocks_count
> / 100.0);
>
> diff --git a/resize/resize2fs.c b/resize/resize2fs.c
> index 216a626..4b83ca9 100644
> --- a/resize/resize2fs.c
> +++ b/resize/resize2fs.c
> @@ -499,18 +499,8 @@ retry:
> ext2fs_bg_flags_zap(fs, i);
> if (csum_flag)
> ext2fs_bg_flags_set(fs, i, EXT2_BG_INODE_UNINIT | EXT2_BG_INODE_ZEROED);
> - if (i == fs->group_desc_count-1) {
> - numblocks = (ext2fs_blocks_count(fs->super) -
> - fs->super->s_first_data_block) %
> - fs->super->s_blocks_per_group;
> - if (!numblocks)
> - numblocks = fs->super->s_blocks_per_group;
> - } else {
> - numblocks = fs->super->s_blocks_per_group;
> - if (csum_flag)
> - ext2fs_bg_flags_set(fs, i,
> - EXT2_BG_BLOCK_UNINIT);
> - }
> +
> + numblocks = ext2fs_group_blocks_count(fs, i);
>
> has_super = ext2fs_bg_has_super(fs, i);
> if (has_super) {
>
>
--
Best Wishes
Yongqiang Yang
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
