One major comment about the library functions. The reason why we have functions of the form ext2fs_foo(), ext2fs_foo2(), ext2fs_foo3(), etc. is to preserve backwards compatibility of the ABI. In general, ext2fs_foo2() will have an extra parameter which wasn't in ext2fs_foo(), and ext2fs_foo2() will have a superset of the functionality of ext2fs_foo(). If later we need to add to add another parameter to further extend the functionality of the function, we might have an ext2fs_foo3(), which again will be a supserset of the functionality of ext2fs_foo2(). So in general, we only need to implement ext2fs_fooN(), and then ext2fs_fooX where 0 <= X <= N simply call ext2fs_fooN with the extra parameters defaulted out (usually to 0 or NULL). It also follows that when we create a new function, there's no need to do this. So the fact that you have an ext2fs_inlinedata_iterate() which just calls ext2fs_inline_data_iterate2() is not something you need to do. More seriously, ext2fs_inline_data_iterate3() has an implementation which is completely different from that of ext2fs_inline_data_iterate2(), and that's an immediate red flag. This means that these two functions are semantically different, and so they should have fundamentally different names --- or you need to make ext2fs_inline_data_iterate3() a strict superset of the functionality of ext2fs_inline_data_iterate2(). Also, I note that there are a number of patches which basically do this: - retval = ext2fs_dir_iterate2(current_fs, inode_num, 0, - 0, rmdir_proc, &rds); + if (ext2fs_has_inline_data(current_fs, inode_num)) + retval = ext2fs_inline_data_iterate2(current_fs, inode_num, 0, + 0, rmdir_proc, &rds); + else + retval = ext2fs_dir_iterate2(current_fs, inode_num, 0, + 0, rmdir_proc, &rds); The much better thing to do is to make ext2fs_dir_iterate2() check to see if the inode has inline data, and if so, to call ext2fs_inline_data_iterate2() directly. This has a couple of benefits. The first is it will reduce the number of patches we need to apply. More importantly, it means that existing programs that don't know about inline data, but who happen to use ext2fs_dir_iterate(), will be able to work automatically, without requiring code changes. It's also much cleaner from a conceptual point of view, since the ext2fs_dir_iterate abstraction shouldn't need to expose to the user whether the directory data is inline or in a separate data block. Regards, - Ted -- 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
