On Wed, Jan 4, 2012 at 10:24 AM, Ted Ts'o <tytso@xxxxxxx> wrote: > On Fri, Dec 30, 2011 at 07:00:00PM +0800, Yongqiang Yang wrote: >> + >> + memset(gdp, 0, EXT4_DESC_SIZE(sb)); >> + /* LV FIXME */ >> + memset(gdp, 0, EXT4_DESC_SIZE(sb)); >> + ext4_block_bitmap_set(sb, gdp, input->block_bitmap); /* LV FIXME */ >> + ext4_inode_bitmap_set(sb, gdp, input->inode_bitmap); /* LV FIXME */ >> + ext4_inode_table_set(sb, gdp, input->inode_table); /* LV FIXME */ > > There is a duplicated (and unneeded) call to memset above. Also, the > LV FIXME was a comment introduced by Laurent Vivier in commit bd81d8ee > because at the time input->block_bitmap was a 32-bit value. In the > new ext4_new_group_data structure, input->block_bitmap is now a 64-bit > field, so the need for "LV FIXME" is gone, and should be removed. > > I'll remove the duplicate memset() calls and the "LV FIXME". > > For future reference, this is why it's better to use something > descriptive about the FIXME "64-bit FIXME", as opposed to your > initials. And it's best if there's an explicit comment explaining why > the fixme is there (and what the consequences of leaving partially > broken code in the kernel :-), so that future code maintainers won't > have to do code archeology to figure out what the FIXME is all about. Thanks for your explanation. I did not figure out what 'LV FIXME' means, so I left them. Now I am understood. Yongqiang. > > - Ted -- 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
