Re: [PATCH] Update the exportfs documentation and comments and fix some exportfs issues

[Christoph Hellwig <hch@infradead.org>]

On Tue, Dec 02, 2008 at 09:02:33PM +0000, David Howells wrote:
> Update the exportfs documentation and comments to fit the code, and add an
> extra FID type to be used for error indication (FILEID_ERROR).
> 
> The following other code changes have been made:
> 
>  (*) The encode_fh() op and exportfs_encode_fh() return enum fid_type rather
>      than int.

Once you change the prototype please also pass the struct fid * instead
of the current __u32 array, like the fh_to_dentry / fh_to_parent
methods.

>  (*) The exportfs_encode_fh() function has it's acceptable() function pointer
>      typedef'd.

I don't really see the point for that, but I don't care too much.

>  (*) The fh_to_dentry() and fh_to_parent() ops and exportfs_decode_fh() take
>      enum fid_type rather than int.

Ok.

>  (*) The generic_fh_to_dentry() and generic_fh_to_parent() functions now take
>      enum fid_type rather than int.  Their get_inode() function pointer has
>      been typedef'd.

Ok.  Again I don't really see the point of the typede, but hey..

>  (*) The generic_fh_to_dentry() and generic_fh_to_parent() functions no longer
>      return NULL, but return -ESTALE instead.  exportfs_decode_fh() does not
>      check for NULL, but will try to dereference it as IS_ERR() does not check
>      for it.

>  (*) The following filesystems have had their fh_to_dentry() and fh_to_parent()
>      implementations made to return -ESTALE rather than NULL: FUSE, GFS2,
>      ISOFS, OCFS2, UDF and XFS.

These two should probably be broken out into a smaller patch, before all
the cosmetic bits.

>  (*) Returns from encode_fh() implementations of 255 are changed to
>      FILEID_ERROR.

>  (*) FAT has had its fh type #defined (FAT_FILEID).
> 
>  (*) FUSE has had its fh types #defined (FUSE_FILEID, FUSE_FILEID_PARENT).
> 
>  (*) ISOFS has had its fh types #defined (ISOFS_FILEID, ISOFS_FILEID_PARENT).
> 
>  (*) OCFS2 has had its fh types #defined (OCFS2_FILEID, OCFS2_FILEID_PARENT).

Ok.


> +A filehandle fragment consists of an array of 1 or more 32-bit words.  The
> +contents and the layout of the data in the array are entirely at the whim of
> +the filesystem that generated it.

Not really true anymore, as we have a real struct for it now.  But all
the length calculation is still done in number of 4 byte words.  Messy..

>  struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
> -		int fh_len, int fileid_type,
> -		int (*acceptable)(void *, struct dentry *), void *context)
> +				  int fh_len, enum fid_type fileid_type,
> +				  exportfs_acceptable_t acceptable,
> +				  void *context)

Why the strange indentation?  Two tabs indent for spillover prototypes
is pretty much standard..

> +#define FAT_FILEID	((enum fid_type) 3)

That cast is completely pointless in C.

> +++ b/include/linux/exportfs.h
> @@ -1,3 +1,9 @@
> +/* Persistent file handle encoding and decoding interface

/*
 * Persistent file handle encoding and decoding interface