On 2011-07-17, at 8:52 PM, Yongqiang Yang wrote:
> add_new_gdb() only needs the no. of a group, there is no need to pass
> a pointer to struct ext4_new_group_data to add_new_gdb().
>
> static int add_new_gdb(handle_t *handle, struct inode *inode,
> - struct ext4_new_group_data *input,
> - struct buffer_head **primary)
> + ext4_group_t group)
This patch is changing the code in a subtle, but not incorrect way.
At a minimum, the commit comment should also mention why "primary"
can be removed as an argument. That is because add_new_gdb() is
storing the bh pointer for the new group descriptor block into
s_group_desc[] internally:
> @@ -513,7 +514,7 @@ static int add_new_gdb(handle_t *handle, struct inode *inode,
> o_group_desc = EXT4_SB(sb)->s_group_desc;
> memcpy(n_group_desc, o_group_desc,
> EXT4_SB(sb)->s_gdb_count * sizeof(struct buffer_head *));
> - n_group_desc[gdb_num] = *primary;
> + n_group_desc[gdb_num] = gdb_bh;
Here...
> @@ -843,8 +844,12 @@ int ext4_group_add(struct super_block *sb, struct ext4_new_group_data *input)
> if (reserved_gdb && ext4_bg_num_gdb(sb, input->group) &&
> (err = reserve_backup_gdb(handle, inode, input)))
> goto exit_journal;
> - } else if ((err = add_new_gdb(handle, inode, input, &primary)))
> - goto exit_journal;
> + } else {
> + err = add_new_gdb(handle, inode, input->group);
> + if (err)
> + goto exit_journal;
> + primary = sbi->s_group_desc[gdb_num];
and the caller can safely access this after the function returns. It
would be incorrect to "optimize" this code to consolidate the setting
of "primary" from sbi->s_group_desc[gdb_num] before the if/else clause,
so this should get a comment that this array element is only valid after
add_new_gdb() returns.
Cheers, Andreas
--
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
