linux-kernel - Re: [PATCH] fs: befs: Replace befs_bread by sb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <576E6C5E.5030602@osg.samsung.com>
Date:	Sat, 25 Jun 2016 12:34:54 +0100
From:	Luis de Bethencourt <luisbg@....samsung.com>
To:	Salah Triki <salah.triki@...il.com>, akpm@...ux-foundation.org
CC:	viro@...iv.linux.org.uk, hannes@...xchg.org,
	vdavydov@...tuozzo.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fs: befs: Replace befs_bread by sb_bread

On 24/06/16 21:30, Salah Triki wrote:
> Since befs_bread merely calls sb_bread, replace it by sb_bread.
> 
> Signed-off-by: Salah Triki <salah.triki@...il.com>
> ---
>  fs/befs/datastream.c |  6 +++---
>  fs/befs/io.c         | 24 ------------------------
>  fs/befs/io.h         |  2 --
>  fs/befs/linuxvfs.c   |  2 +-
>  4 files changed, 4 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/befs/datastream.c b/fs/befs/datastream.c
> index af1bc19..26cc417 100644
> --- a/fs/befs/datastream.c
> +++ b/fs/befs/datastream.c
> @@ -326,7 +326,7 @@ befs_find_brun_indirect(struct super_block *sb,
>  
>  	/* Examine blocks of the indirect run one at a time */
>  	for (i = 0; i < indirect.len; i++) {
> -		indirblock = befs_bread(sb, indirblockno + i);
> +		indirblock = sb_bread(sb, indirblockno + i);
>  		if (indirblock == NULL) {
>  			befs_debug(sb, "---> %s failed to read "
>  				   "disk block %lu from the indirect brun",
> @@ -471,7 +471,7 @@ befs_find_brun_dblindirect(struct super_block *sb,
>  	}
>  
>  	dbl_indir_block =
> -	    befs_bread(sb, iaddr2blockno(sb, &data->double_indirect) +
> +	    sb_bread(sb, iaddr2blockno(sb, &data->double_indirect) +
>  					dbl_which_block);
>  	if (dbl_indir_block == NULL) {
>  		befs_error(sb, "%s couldn't read the "
> @@ -499,7 +499,7 @@ befs_find_brun_dblindirect(struct super_block *sb,
>  	}
>  
>  	indir_block =
> -	    befs_bread(sb, iaddr2blockno(sb, &indir_run) + which_block);
> +	    sb_bread(sb, iaddr2blockno(sb, &indir_run) + which_block);
>  	if (indir_block == NULL) {
>  		befs_error(sb, "%s couldn't read the indirect block "
>  			   "at blockno %lu", __func__, (unsigned long)
> diff --git a/fs/befs/io.c b/fs/befs/io.c
> index 523c8af..4223b77 100644
> --- a/fs/befs/io.c
> +++ b/fs/befs/io.c
> @@ -59,27 +59,3 @@ befs_bread_iaddr(struct super_block *sb, befs_inode_addr iaddr)
>  	befs_debug(sb, "<--- %s ERROR", __func__);
>  	return NULL;
>  }
> -
> -struct buffer_head *
> -befs_bread(struct super_block *sb, befs_blocknr_t block)
> -{
> -	struct buffer_head *bh;
> -
> -	befs_debug(sb, "---> Enter %s %lu", __func__, (unsigned long)block);
> -
> -	bh = sb_bread(sb, block);
> -
> -	if (bh == NULL) {
> -		befs_error(sb, "Failed to read block %lu",
> -			   (unsigned long)block);
> -		goto error;
> -	}
> -
> -	befs_debug(sb, "<--- %s", __func__);
> -
> -	return bh;
> -
> -      error:
> -	befs_debug(sb, "<--- %s ERROR", __func__);
> -	return NULL;
> -}
> diff --git a/fs/befs/io.h b/fs/befs/io.h
> index 9b78266..78d7bc6 100644
> --- a/fs/befs/io.h
> +++ b/fs/befs/io.h
> @@ -5,5 +5,3 @@
>  struct buffer_head *befs_bread_iaddr(struct super_block *sb,
>  				     befs_inode_addr iaddr);
>  
> -struct buffer_head *befs_bread(struct super_block *sb, befs_blocknr_t block);
> -
> diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
> index 2fea87b..6bc5b40 100644
> --- a/fs/befs/linuxvfs.c
> +++ b/fs/befs/linuxvfs.c
> @@ -319,7 +319,7 @@ static struct inode *befs_iget(struct super_block *sb, unsigned long ino)
>  		   befs_ino->i_inode_num.allocation_group,
>  		   befs_ino->i_inode_num.start, befs_ino->i_inode_num.len);
>  
> -	bh = befs_bread(sb, inode->i_ino);
> +	bh = sb_bread(sb, inode->i_ino);
>  	if (!bh) {
>  		befs_error(sb, "unable to read inode block - "
>  			   "inode = %lu", inode->i_ino);
> 

This simplifies the code.

The only thing you lose is this line:
befs_debug(sb, "---> Enter %s %lu", __func__, (unsigned long)block);

Which doesn't have much use when the function doesn't exist.

All calls of befs_bread() later check if the return is NULL and write a
befs_error() if its. So you keep all error checking, which is important.

I think it is a good idea. I don't foresee new calls to befs_bread() added
that would justify its existence.

Acked-by: Luis de Bethencourt <luisbg@....samsung.com>

Thanks Salah,
Luis