[PATCH 1/6] fs: befs: remove unneeded initialization to zero

[PATCH 1/6] fs: befs: remove unneeded initialization to zero

[Salah Triki]

off is reinitialized by befs_read_datastream, so no need to init it with
zero in the beginning of befs_bt_read_node.

Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
 fs/befs/btree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/befs/btree.c b/fs/befs/btree.c
index e59ad20..a0e8cfa 100644
--- a/fs/befs/btree.c
+++ b/fs/befs/btree.c
@@ -196,7 +196,7 @@ static int
 befs_bt_read_node(struct super_block *sb, const befs_data_stream *ds,
 		  struct befs_btree_node *node, befs_off_t node_off)
 {
-	uint off = 0;
+	uint off;
 
 	befs_debug(sb, "---> %s", __func__);
 
-- 
1.9.1

[PATCH 2/6] fs: befs: remove in vain variable assignment

[Salah Triki]

There is no need to set *value, it will be overwritten later.

Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
 fs/befs/btree.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/befs/btree.c b/fs/befs/btree.c
index a0e8cfa..f33fc6c 100644
--- a/fs/befs/btree.c
+++ b/fs/befs/btree.c
@@ -348,8 +348,6 @@ befs_find_key(struct super_block *sb, struct befs_btree_node *node,
 
 	befs_debug(sb, "---> %s %s", __func__, findkey);
 
-	*value = 0;
-
 	findkey_len = strlen(findkey);
 
 	/* if node can not contain key, just skeep this node */
-- 
1.9.1

[PATCH 3/6] fs: befs: remove useless initialization to zero

[Salah Triki]

node_off is unconditionally set to bt_super.root_node_ptr, so no need to
init it to zero.

Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
 fs/befs/btree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/befs/btree.c b/fs/befs/btree.c
index f33fc6c..3cb97e8 100644
--- a/fs/befs/btree.c
+++ b/fs/befs/btree.c
@@ -420,7 +420,7 @@ befs_btree_read(struct super_block *sb, const befs_data_stream *ds,
 {
 	struct befs_btree_node *this_node;
 	befs_btree_super bt_super;
-	befs_off_t node_off = 0;
+	befs_off_t node_off;
 	int cur_key;
 	fs64 *valarray;
 	char *keystart;
-- 
1.9.1

[PATCH 4/6] fs: befs: remove unnecessary *befs_sb variable

[Salah Triki]

Remove *befs_sb and just call BEFS_SB(sb) directly, since the returned
value by this function is only used once.

Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
 fs/befs/datastream.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/befs/datastream.c b/fs/befs/datastream.c
index b68b6f9..343123c 100644
--- a/fs/befs/datastream.c
+++ b/fs/befs/datastream.c
@@ -422,10 +422,9 @@ befs_find_brun_dblindirect(struct super_block *sb,
 	struct buffer_head *indir_block;
 	befs_block_run indir_run;
 	befs_disk_inode_addr *iaddr_array;
-	struct befs_sb_info *befs_sb = BEFS_SB(sb);
 
 	befs_blocknr_t indir_start_blk =
-	    data->max_indirect_range >> befs_sb->block_shift;
+	    data->max_indirect_range >> BEFS_SB(sb)->block_shift;
 
 	off_t dbl_indir_off = blockno - indir_start_blk;
 
-- 
1.9.1

[PATCH 5/6] fs: befs: remove in vain variable assignment

[Salah Triki]

There is no need to init res, since it will be overwitten later by
befs_fblock2brun().

Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
 fs/befs/linuxvfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
index 6bc5b40..2b68c81 100644
--- a/fs/befs/linuxvfs.c
+++ b/fs/befs/linuxvfs.c
@@ -120,7 +120,7 @@ befs_get_block(struct inode *inode, sector_t block,
 	struct super_block *sb = inode->i_sb;
 	befs_data_stream *ds = &BEFS_I(inode)->i_data.ds;
 	befs_block_run run = BAD_IADDR;
-	int res = 0;
+	int res;
 	ulong disk_off;
 
 	befs_debug(sb, "---> befs_get_block() for inode %lu, block %ld",
-- 
1.9.1

[PATCH 6/6] fs: befs: remove ret variable

[Salah Triki]

ret is initialized to -EIO and is never modified, so remove ret and use
-EIO directly.

Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
 fs/befs/linuxvfs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
index 2b68c81..c57f831 100644
--- a/fs/befs/linuxvfs.c
+++ b/fs/befs/linuxvfs.c
@@ -300,7 +300,6 @@ static struct inode *befs_iget(struct super_block *sb, unsigned long ino)
 	struct befs_sb_info *befs_sb = BEFS_SB(sb);
 	struct befs_inode_info *befs_ino;
 	struct inode *inode;
-	long ret = -EIO;
 
 	befs_debug(sb, "---> %s inode = %lu", __func__, ino);
 
@@ -422,7 +421,7 @@ static struct inode *befs_iget(struct super_block *sb, unsigned long ino)
       unacquire_none:
 	iget_failed(inode);
 	befs_debug(sb, "<--- %s - Bad inode", __func__);
-	return ERR_PTR(ret);
+	return ERR_PTR(-EIO);
 }
 
 /* Initialize the inode cache. Called at fs setup.
-- 
1.9.1

Re: [PATCH 1/6] fs: befs: remove unneeded initialization to zero

[Luis de Bethencourt]

On 31/07/16 21:34, Salah Triki wrote:
> off is reinitialized by befs_read_datastream, so no need to init it with
> zero in the beginning of befs_bt_read_node.
> 
> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> ---
>  fs/befs/btree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/befs/btree.c b/fs/befs/btree.c
> index e59ad20..a0e8cfa 100644
> --- a/fs/befs/btree.c
> +++ b/fs/befs/btree.c
> @@ -196,7 +196,7 @@ static int
>  befs_bt_read_node(struct super_block *sb, const befs_data_stream *ds,
>  		  struct befs_btree_node *node, befs_off_t node_off)
>  {
> -	uint off = 0;
> +	uint off;
>  
>  	befs_debug(sb, "---> %s", __func__);
>  
> 

Hi Salah,

I will quote Andrew here:
"With this code:

	int foo;

	bar(&foo);

	whatever = foo;

some versions of gcc will warn that foo might be used uninitialized. 
Other versions of gcc don't do this.  That's why the seemingly-unneeded
initializations are there."

You can read the rest of his reply to when I sent the same change 2
months ago :)

https://lkml.org/lkml/2016/6/1/875

Sorry, I agree with him to keep the code as it is.

Nacked.

Thanks,
Luis

Re: [PATCH 2/6] fs: befs: remove in vain variable assignment

[Luis de Bethencourt]

On 31/07/16 21:34, Salah Triki wrote:
> There is no need to set *value, it will be overwritten later.
> 
> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> ---
>  fs/befs/btree.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/befs/btree.c b/fs/befs/btree.c
> index a0e8cfa..f33fc6c 100644
> --- a/fs/befs/btree.c
> +++ b/fs/befs/btree.c
> @@ -348,8 +348,6 @@ befs_find_key(struct super_block *sb, struct befs_btree_node *node,
>  
>  	befs_debug(sb, "---> %s %s", __func__, findkey);
>  
> -	*value = 0;
> -
>  	findkey_len = strlen(findkey);
>  
>  	/* if node can not contain key, just skeep this node */
> 

Hi Salah,

The key here is that befs_btree_find(), the only consumer of befs_find_key(),
doesn't use the value if the return is BEFS_BT_NOT_FOUND.

Tested the patch anyway to be sure.

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

Pushed to the befs-next branch:
https://github.com/luisbg/linux-befs/tree/befs-next

Thanks!
Luis

Re: [PATCH 3/6] fs: befs: remove useless initialization to zero

[Luis de Bethencourt]

On 31/07/16 21:34, Salah Triki wrote:
> node_off is unconditionally set to bt_super.root_node_ptr, so no need to
> init it to zero.
> 
> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> ---
>  fs/befs/btree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/befs/btree.c b/fs/befs/btree.c
> index f33fc6c..3cb97e8 100644
> --- a/fs/befs/btree.c
> +++ b/fs/befs/btree.c
> @@ -420,7 +420,7 @@ befs_btree_read(struct super_block *sb, const befs_data_stream *ds,
>  {
>  	struct befs_btree_node *this_node;
>  	befs_btree_super bt_super;
> -	befs_off_t node_off = 0;
> +	befs_off_t node_off;
>  	int cur_key;
>  	fs64 *valarray;
>  	char *keystart;
> 

Looks good to me.

Strange that static analysis didn't pick this one up before.

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

Pushed to the befs-next branch:
https://github.com/luisbg/linux-befs/tree/befs-next

Thanks Salah,
Luis

Re: [PATCH 4/6] fs: befs: remove unnecessary *befs_sb variable

[Luis de Bethencourt]

On 31/07/16 21:34, Salah Triki wrote:
> Remove *befs_sb and just call BEFS_SB(sb) directly, since the returned
> value by this function is only used once.
> 
> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> ---
>  fs/befs/datastream.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/befs/datastream.c b/fs/befs/datastream.c
> index b68b6f9..343123c 100644
> --- a/fs/befs/datastream.c
> +++ b/fs/befs/datastream.c
> @@ -422,10 +422,9 @@ befs_find_brun_dblindirect(struct super_block *sb,
>  	struct buffer_head *indir_block;
>  	befs_block_run indir_run;
>  	befs_disk_inode_addr *iaddr_array;
> -	struct befs_sb_info *befs_sb = BEFS_SB(sb);
>  
>  	befs_blocknr_t indir_start_blk =
> -	    data->max_indirect_range >> befs_sb->block_shift;
> +	    data->max_indirect_range >> BEFS_SB(sb)->block_shift;
>  
>  	off_t dbl_indir_off = blockno - indir_start_blk;
>  
> 

This looks to be consistent with other uses of BEFS_SB() when the value is
only used once.

Thanks,
Luis

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

Pushed to the befs-next branch:
https://github.com/luisbg/linux-befs/tree/befs-next

Re: [PATCH 5/6] fs: befs: remove in vain variable assignment

[Luis de Bethencourt]

On 31/07/16 21:34, Salah Triki wrote:
> There is no need to init res, since it will be overwitten later by
> befs_fblock2brun().
> 
> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> ---
>  fs/befs/linuxvfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
> index 6bc5b40..2b68c81 100644
> --- a/fs/befs/linuxvfs.c
> +++ b/fs/befs/linuxvfs.c
> @@ -120,7 +120,7 @@ befs_get_block(struct inode *inode, sector_t block,
>  	struct super_block *sb = inode->i_sb;
>  	befs_data_stream *ds = &BEFS_I(inode)->i_data.ds;
>  	befs_block_run run = BAD_IADDR;
> -	int res = 0;
> +	int res;
>  	ulong disk_off;
>  
>  	befs_debug(sb, "---> befs_get_block() for inode %lu, block %ld",
> 

Looks good.

Thanks Salah,
Luis

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

Pushed to the befs-next branch:
https://github.com/luisbg/linux-befs/tree/befs-next

Re: [PATCH 6/6] fs: befs: remove ret variable

[Luis de Bethencourt]

On 31/07/16 21:34, Salah Triki wrote:
> ret is initialized to -EIO and is never modified, so remove ret and use
> -EIO directly.
> 
> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> ---
>  fs/befs/linuxvfs.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
> index 2b68c81..c57f831 100644
> --- a/fs/befs/linuxvfs.c
> +++ b/fs/befs/linuxvfs.c
> @@ -300,7 +300,6 @@ static struct inode *befs_iget(struct super_block *sb, unsigned long ino)
>  	struct befs_sb_info *befs_sb = BEFS_SB(sb);
>  	struct befs_inode_info *befs_ino;
>  	struct inode *inode;
> -	long ret = -EIO;
>  
>  	befs_debug(sb, "---> %s inode = %lu", __func__, ino);
>  
> @@ -422,7 +421,7 @@ static struct inode *befs_iget(struct super_block *sb, unsigned long ino)
>        unacquire_none:
>  	iget_failed(inode);
>  	befs_debug(sb, "<--- %s - Bad inode", __func__);
> -	return ERR_PTR(ret);
> +	return ERR_PTR(-EIO);
>  }
>  
>  /* Initialize the inode cache. Called at fs setup.
> 

Hi Salah,

This one is a nice catch, since your patch also makes the code more readable.
No need to check the value of ret when you read that return ERR_PTR() anymore.

This has been this way since the introduction of ret in commit
96eb5419412fbc7f39fa45d987034c5d0e6e1202.
No need to add a "Fixes" tag in the commit message because that commit was 8
years ago.

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

Thanks :)
Luis

Pushed to the befs-next branch:
https://github.com/luisbg/linux-befs/tree/befs-next

Re: [PATCH 2/6] fs: befs: remove in vain variable assignment

[Salah Triki]

On Mon, Aug 01, 2016 at 02:24:34PM +0100, Luis de Bethencourt wrote:
> On 31/07/16 21:34, Salah Triki wrote:
> > There is no need to set *value, it will be overwritten later.
> > 
> > Signed-off-by: Salah Triki <salah.triki@gmail.com>
> > ---
> >  fs/befs/btree.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/fs/befs/btree.c b/fs/befs/btree.c
> > index a0e8cfa..f33fc6c 100644
> > --- a/fs/befs/btree.c
> > +++ b/fs/befs/btree.c
> > @@ -348,8 +348,6 @@ befs_find_key(struct super_block *sb, struct befs_btree_node *node,
> >  
> >  	befs_debug(sb, "---> %s %s", __func__, findkey);
> >  
> > -	*value = 0;
> > -
> >  	findkey_len = strlen(findkey);
> >  
> >  	/* if node can not contain key, just skeep this node */
> > 
> 
> Hi Salah,
> 
> The key here is that befs_btree_find(), the only consumer of befs_find_key(),
> doesn't use the value if the return is BEFS_BT_NOT_FOUND.
> 
> Tested the patch anyway to be sure.
> 
> Acked-by: Luis de Bethencourt <luisbg@osg.samsung.com>
> 
> Pushed to the befs-next branch:
> https://github.com/luisbg/linux-befs/tree/befs-next
> 
> Thanks!
> Luis

Thanx :)

Salah

Re: [PATCH 3/6] fs: befs: remove useless initialization to zero

[Salah Triki]

On Mon, Aug 01, 2016 at 02:44:18PM +0100, Luis de Bethencourt wrote:
> On 31/07/16 21:34, Salah Triki wrote:
> > node_off is unconditionally set to bt_super.root_node_ptr, so no need to
> > init it to zero.
> > 
> > Signed-off-by: Salah Triki <salah.triki@gmail.com>
> > ---
> >  fs/befs/btree.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/befs/btree.c b/fs/befs/btree.c
> > index f33fc6c..3cb97e8 100644
> > --- a/fs/befs/btree.c
> > +++ b/fs/befs/btree.c
> > @@ -420,7 +420,7 @@ befs_btree_read(struct super_block *sb, const befs_data_stream *ds,
> >  {
> >  	struct befs_btree_node *this_node;
> >  	befs_btree_super bt_super;
> > -	befs_off_t node_off = 0;
> > +	befs_off_t node_off;
> >  	int cur_key;
> >  	fs64 *valarray;
> >  	char *keystart;
> > 
> 
> Looks good to me.
> 
> Strange that static analysis didn't pick this one up before.
> 
> Acked-by: Luis de Bethencourt <luisbg@osg.samsung.com>
> 
> Pushed to the befs-next branch:
> https://github.com/luisbg/linux-befs/tree/befs-next
> 
> Thanks Salah,
> Luis

Thanx :)
Salah

Re: [PATCH 4/6] fs: befs: remove unnecessary *befs_sb variable

[Salah Triki]

On Mon, Aug 01, 2016 at 03:02:57PM +0100, Luis de Bethencourt wrote:
> On 31/07/16 21:34, Salah Triki wrote:
> > Remove *befs_sb and just call BEFS_SB(sb) directly, since the returned
> > value by this function is only used once.
> > 
> > Signed-off-by: Salah Triki <salah.triki@gmail.com>
> > ---
> >  fs/befs/datastream.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/fs/befs/datastream.c b/fs/befs/datastream.c
> > index b68b6f9..343123c 100644
> > --- a/fs/befs/datastream.c
> > +++ b/fs/befs/datastream.c
> > @@ -422,10 +422,9 @@ befs_find_brun_dblindirect(struct super_block *sb,
> >  	struct buffer_head *indir_block;
> >  	befs_block_run indir_run;
> >  	befs_disk_inode_addr *iaddr_array;
> > -	struct befs_sb_info *befs_sb = BEFS_SB(sb);
> >  
> >  	befs_blocknr_t indir_start_blk =
> > -	    data->max_indirect_range >> befs_sb->block_shift;
> > +	    data->max_indirect_range >> BEFS_SB(sb)->block_shift;
> >  
> >  	off_t dbl_indir_off = blockno - indir_start_blk;
> >  
> > 
> 
> This looks to be consistent with other uses of BEFS_SB() when the value is
> only used once.
> 
> Thanks,
> Luis
> 
> Acked-by: Luis de Bethencourt <luisbg@osg.samsung.com>
> 
> Pushed to the befs-next branch:
> https://github.com/luisbg/linux-befs/tree/befs-next

Thanx :)
salah