XFS_ERROR use - was Re: [PATCH] prevent NULL returns from d_obtain_alias

[Timothy Shimmin <tes@sgi.com>]

Christoph Hellwig wrote:
> On Thu, Oct 16, 2008 at 07:50:10PM +0200, Miklos Szeredi wrote:
>> should be
>>
>>   		return -XFS_ERROR(-PTR_ERR(dentry));
> 
> No need for XFS_ERROR at all here, it's only used for error injection in
> low-level code.  Updated version that propagates the error below:
> 
> (Al, if you rebase vfs.git this should probably be folded into the patch
> that switches over to d_obtain_alias)
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 

[...stuff deleted]

> Index: vfs-2.6/fs/xfs/linux-2.6/xfs_ioctl.c
> ===================================================================
> --- vfs-2.6.orig/fs/xfs/linux-2.6/xfs_ioctl.c	2008-10-15 21:26:33.000000000 +0200
> +++ vfs-2.6/fs/xfs/linux-2.6/xfs_ioctl.c	2008-10-16 20:06:20.000000000 +0200
> @@ -312,9 +312,9 @@ xfs_open_by_handle(
>  	}
>  
>  	dentry = d_obtain_alias(inode);
> -	if (dentry == NULL) {
> +	if (IS_ERR(dentry)) {
>  		put_unused_fd(new_fd);
> -		return -XFS_ERROR(ENOMEM);
> +		return PTR_ERR(dentry);
>  	}
>  
>  	/* Ensure umount returns EBUSY on umounts while this file is open. */
> --

Fair enough.
But XFS_ERROR is used throughout the function.
I've found the whole idea of when and when not to use XFS_ERROR annoying :)

I've never used it (other than calling it to stay consistent with the code).
Looking at the code, it is used to BUG and print a msg on particular error codes set in xfs_etrap[] -
and it does this in xfs_error_trap().
Can one not decide to do this at any error point?
I can't see where we hook in to set up xfs_etrap.

> #ifdef DEBUG
> #define XFS_ERROR_NTRAP 10
> extern int      xfs_etrap[XFS_ERROR_NTRAP];
> extern int      xfs_error_trap(int);
> #define XFS_ERROR(e)    xfs_error_trap(e)
> #else
> #define XFS_ERROR(e)    (e)
> #endif


Cheers,
Tim.