On Jun 05, 2008 11:37 -0400, Theodore Ts'o wrote: > This is better, but it still means that we are exporting a large > number of functions to the callers. It's not clear to me we need so > many different variants of ext4_new_blocks_* --- what is their > justification to exist? > > For example, why not just have: > > static ext4_fsblk_t ext4_new_blocks(handle_t *handle, struct inode *inode, > ext4_lblk_t iblock, ext4_fsblk_t goal, > unsigned long *count, int *errp, int meta) > > where if inode is NULL, then you're allocating a metadata block, and > if count is NULL, then you only want one block. Of course, this needs > to be carefully documented at the function. I don't necessarily agree that meta should be implied by inode != NULL. We do want to cluster metadata allocations for a single inode if possible, so keeping the inode information is useful. We may want to keep a separate "metadata goal block" from the "data goal block" in the inode... That said, it seems you still have a "meta" parameter here? I always hate having an int for a boolean, and we may as well make this a "flags" so that when we want to improve it later we don't need to rename it and change all of the "1" parameters to "EXT4_META_BLOCK". Do it right the first time. Cheers, Andreas -- Andreas Dilger Sr. Staff Engineer, Lustre Group Sun Microsystems of Canada, Inc. -- 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
