| Message ID | 20190808082744.31405-5-cmaiolino@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | New ->fiemap infrastructure and ->bmap removal | expand |
Hey folks, > All errors (new ones prefixed by >>): > > fs/ioctl.c: In function 'ioctl_fibmap': > >> fs/ioctl.c:68:10: error: implicit declaration of function 'bmap'; did you mean 'kmap'? [-Werror=implicit-function-declaration] Any of you guys may have a better idea on how to fix this? Essentially, this happens when CONFIG_BLOCK is not set, and although I don't really see a hard requirement to have bmap() exported only when CONFIG_BLOCK is set, at the same time, I don't see use for bmap() if CONFIG_BLOCK is not set. So, I'm in a kind of a chicken-egg problem. I am considering to just remove the #ifdef CONFIG_BLOCK / #endif from the bmap() declaration. This will fix the warning, and I don't see any side effects. What you guys think? > error = bmap(inode, &block); > ^~~~ > kmap > cc1: some warnings being treated as errors > > vim +68 fs/ioctl.c > > 53 > 54 static int ioctl_fibmap(struct file *filp, int __user *p) > 55 { > 56 struct inode *inode = file_inode(filp); > 57 int error, ur_block; > 58 sector_t block; > 59 > 60 if (!capable(CAP_SYS_RAWIO)) > 61 return -EPERM; > 62 > 63 error = get_user(ur_block, p); > 64 if (error) > 65 return error; > 66 > 67 block = ur_block; > > 68 error = bmap(inode, &block); > 69 > 70 if (error) > 71 ur_block = 0; > 72 else > 73 ur_block = block; > 74 > 75 error = put_user(ur_block, p); > 76 > 77 return error; > 78 } > 79 > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Wed, Aug 14, 2019 at 01:01:50PM +0200, Carlos Maiolino wrote: > Hey folks, > > > All errors (new ones prefixed by >>): > > > > fs/ioctl.c: In function 'ioctl_fibmap': > > >> fs/ioctl.c:68:10: error: implicit declaration of function 'bmap'; did you mean 'kmap'? [-Werror=implicit-function-declaration] > > Any of you guys may have a better idea on how to fix this? > > Essentially, this happens when CONFIG_BLOCK is not set, and although I don't > really see a hard requirement to have bmap() exported only when CONFIG_BLOCK is > set, at the same time, I don't see use for bmap() if CONFIG_BLOCK is not set. > > So, I'm in a kind of a chicken-egg problem. > > I am considering to just remove the #ifdef CONFIG_BLOCK / #endif from the bmap() > declaration. This will fix the warning, and I don't see any side effects. What > you guys think? Just provide an inline !CONFIG_BLOCK stub for bmap() in fs.h that always returns -EINVAL.
diff --git a/fs/ioctl.c b/fs/ioctl.c index fef3a6bf7c78..6b589c873bc2 100644 --- a/fs/ioctl.c +++ b/fs/ioctl.c @@ -53,19 +53,28 @@ EXPORT_SYMBOL(vfs_ioctl); static int ioctl_fibmap(struct file *filp, int __user *p) { - struct address_space *mapping = filp->f_mapping; - int res, block; + struct inode *inode = file_inode(filp); + int error, ur_block; + sector_t block; - /* do we support this mess? */ - if (!mapping->a_ops->bmap) - return -EINVAL; if (!capable(CAP_SYS_RAWIO)) return -EPERM; - res = get_user(block, p); - if (res) - return res; - res = mapping->a_ops->bmap(mapping, block); - return put_user(res, p); + + error = get_user(ur_block, p); + if (error) + return error; + + block = ur_block; + error = bmap(inode, &block); + + if (error) + ur_block = 0; + else + ur_block = block; + + error = put_user(ur_block, p); + + return error; } /**
Now we have the possibility of proper error return in bmap, use bmap() function in ioctl_fibmap() instead of calling ->bmap method directly. Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> --- Changelog: V4: - Ensure ioctl_fibmap() returns 0 in case of error returned from bmap(). Otherwise we'll be changing the user interface (which returns 0 in case of error) V3: - Rename usr_blk to ur_block V2: - Use a local sector_t variable to asign the block number instead of using direct casting. fs/ioctl.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-)