diff mbox series

[3/3] ecryptfs: drop direct calls to ->bmap

Message ID 20180912122536.31977-4-cmaiolino@redhat.com (mailing list archive)
State New, archived
Headers show
Series Replace direct ->bmap calls by bmap() with error support | expand

Commit Message

Carlos Maiolino Sept. 12, 2018, 12:25 p.m. UTC
Replace direct ->bmap calls by bmap() method.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/ecryptfs/mmap.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Christoph Hellwig Sept. 14, 2018, 1:26 p.m. UTC | #1
On Wed, Sep 12, 2018 at 02:25:36PM +0200, Carlos Maiolino wrote:
>  static sector_t ecryptfs_bmap(struct address_space *mapping, sector_t block)
>  {
> +	sector_t blk_map = 0;
> +	int ret;
>  	struct inode *inode;
>  	struct inode *lower_inode;
>  
>  	inode = (struct inode *)mapping->host;
>  	lower_inode = ecryptfs_inode_to_lower(inode);
> +
> +	ret = bmap(lower_inode, &blk_map);
> +
> +	return !ret ? blk_map : 0;

This could be simplified to:

static sector_t ecryptfs_bmap(struct address_space *mapping, sector_t block)
{
 	struct inode *lower_inode = ecryptfs_inode_to_lower(mapping->host);
	int ret = bmap(lower_inode, &block);

	if (ret)
		return 0;
	return block;
}

But the idea that we even support ->bmap on ecryptfs sounds way too
dangerous.
Carlos Maiolino Sept. 14, 2018, 6:47 p.m. UTC | #2
On Fri, Sep 14, 2018 at 03:26:16PM +0200, Christoph Hellwig wrote:
> On Wed, Sep 12, 2018 at 02:25:36PM +0200, Carlos Maiolino wrote:
> >  static sector_t ecryptfs_bmap(struct address_space *mapping, sector_t block)
> >  {
> > +	sector_t blk_map = 0;
> > +	int ret;
> >  	struct inode *inode;
> >  	struct inode *lower_inode;
> >  
> >  	inode = (struct inode *)mapping->host;
> >  	lower_inode = ecryptfs_inode_to_lower(inode);
> > +
> > +	ret = bmap(lower_inode, &blk_map);
> > +
> > +	return !ret ? blk_map : 0;
> 
> This could be simplified to:
> 
> static sector_t ecryptfs_bmap(struct address_space *mapping, sector_t block)
> {
>  	struct inode *lower_inode = ecryptfs_inode_to_lower(mapping->host);
> 	int ret = bmap(lower_inode, &block);
> 
> 	if (ret)
> 		return 0;
> 	return block;
> }

I can change, without problem.
> 
> But the idea that we even support ->bmap on ecryptfs sounds way too
> dangerous.

I can't argue here, I don't know much about ecryptfs, I just replaced the ->bmap
call keeping the same logic, but thanks a lot for the review, I'll update it and
send a V2
diff mbox series

Patch

diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c
index cdf358b209d9..256721eb6a03 100644
--- a/fs/ecryptfs/mmap.c
+++ b/fs/ecryptfs/mmap.c
@@ -538,16 +538,17 @@  static int ecryptfs_write_end(struct file *file,
 
 static sector_t ecryptfs_bmap(struct address_space *mapping, sector_t block)
 {
-	int rc = 0;
+	sector_t blk_map = 0;
+	int ret;
 	struct inode *inode;
 	struct inode *lower_inode;
 
 	inode = (struct inode *)mapping->host;
 	lower_inode = ecryptfs_inode_to_lower(inode);
-	if (lower_inode->i_mapping->a_ops->bmap)
-		rc = lower_inode->i_mapping->a_ops->bmap(lower_inode->i_mapping,
-							 block);
-	return rc;
+
+	ret = bmap(lower_inode, &blk_map);
+
+	return !ret ? blk_map : 0;
 }
 
 const struct address_space_operations ecryptfs_aops = {