Re: [PATCH 04/10 V2] fibmap: Use bmap instead of ->bmap method in ioctl_fibmap

[Christoph Hellwig <hch@lst.de>]

On Wed, Dec 05, 2018 at 10:17:22AM +0100, Carlos Maiolino wrote:
> Now we have the possibility of proper error return in bmap, use bmap()
> function in ioctl_fibmap() instead of calling ->bmap method directly.
> 
> V2:
> 	- Use a local sector_t variable to asign the block number
> 	  instead of using direct casting.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  fs/ioctl.c | 27 +++++++++++++++++----------
>  1 file changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index d64f622cac8b..e0cc0dd5f9aa 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -53,19 +53,26 @@ EXPORT_SYMBOL(vfs_ioctl);
>  
>  static int ioctl_fibmap(struct file *filp, int __user *p)
>  {
> +	struct inode *inode = file_inode(filp);
> +	int error, usr_blk;
> +	sector_t block;
>  
>  	if (!capable(CAP_SYS_RAWIO))
>  		return -EPERM;
> +
> +	error = get_user(usr_blk, p);
> +	if (error)
> +		return error;

Does get_user/put_user actually return an error?

All the code I know just does:

	if (get_user()))
		return -EFAULT;

and co.

> +
> +	block = usr_blk;
> +	error = bmap(inode, &block);
> +	if (error)
> +		return error;
> +	usr_blk = block;

Nitpick: maybe i'd name ur_block block and block sector, which seems
to flow a little better.