On 07/02/2012 07:47 AM, Dave Chinner wrote:
> On Sat, Jun 30, 2012 at 02:06:42AM +0800, Wanlong Gao wrote:
>> s/xfs_fs_/xfs_/
>> This too long name is unnecessary.
>
> NACK. Not the right way to approach the problem.
>
> Yes, there are a couple of places where this could be done, but most
> of the transformations, IMO, are wrong and remove critical
> information from the namespace.
>
> Firstly, we want to get rid of the xfs_fs_subr.c wrappers and
> replace them with the native linux functions, not move them around.
>
> Secondly, transformations like xfs_fs_geometry to xfs_geometry are
> incorrect - the function is returning the filesystem geometry, so
> the name "fs_geometry" is actually correct. Similar for
> xfs_fs_writeable, xfs_fs_show_options and so on.
Thank you for teaching this.
Wanlong Gao
>
> Finally:
>
>> const struct export_operations xfs_export_operations = {
>> - .encode_fh = xfs_fs_encode_fh,
>> - .fh_to_dentry = xfs_fs_fh_to_dentry,
>> - .fh_to_parent = xfs_fs_fh_to_parent,
>> - .get_parent = xfs_fs_get_parent,
>> - .commit_metadata = xfs_fs_nfs_commit_metadata,
>> + .encode_fh = xfs_encode_fh,
>> + .fh_to_dentry = xfs_fh_to_dentry,
>> + .fh_to_parent = xfs_fh_to_parent,
>> + .get_parent = xfs_get_parent,
>> + .commit_metadata = xfs_nfs_commit_metadata,
>
> ....
>
>> static const struct super_operations xfs_super_operations = {
>> - .alloc_inode = xfs_fs_alloc_inode,
>> - .destroy_inode = xfs_fs_destroy_inode,
>> - .dirty_inode = xfs_fs_dirty_inode,
>> - .evict_inode = xfs_fs_evict_inode,
>> - .drop_inode = xfs_fs_drop_inode,
>> - .put_super = xfs_fs_put_super,
>> - .sync_fs = xfs_fs_sync_fs,
>> - .freeze_fs = xfs_fs_freeze,
>> - .unfreeze_fs = xfs_fs_unfreeze,
>> - .statfs = xfs_fs_statfs,
>> - .remount_fs = xfs_fs_remount,
>> - .show_options = xfs_fs_show_options,
>> - .nr_cached_objects = xfs_fs_nr_cached_objects,
>> - .free_cached_objects = xfs_fs_free_cached_objects,
>> + .alloc_inode = xfs_alloc_inode,
>> + .destroy_inode = xfs_destroy_inode,
>> + .dirty_inode = xfs_dirty_inode,
>> + .evict_inode = xfs_evict_inode,
>> + .drop_inode = xfs_drop_inode,
>> + .put_super = xfs_put_super,
>> + .sync_fs = xfs_sync_fs,
>> + .freeze_fs = xfs_freeze,
>> + .unfreeze_fs = xfs_unfreeze,
>> + .statfs = xfs_statfs,
>> + .remount_fs = xfs_remount,
>> + .show_options = xfs_show_options,
>> + .nr_cached_objects = xfs_nr_cached_objects,
>> + .free_cached_objects = xfs_free_cached_objects,
>> };
>
> These should indicate that there is a valid, consistent namespacing
> here to indicate layering of the the code. i.e. that xfs_fs_*
> functions are typically VFS method functions of some kind.
>
>> -static struct file_system_type xfs_fs_type = {
>> +static struct file_system_type xfs_type = {
>
> And that is a clear demonstration of my second point....
>
> Cheers,
>
> Dave.
>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
