diff mbox

[4/4] UBIFS: Remove useless statements

Message ID 5397C22C.70305@huawei.com
State Rejected
Headers show

Commit Message

hujianyang June 11, 2014, 2:42 a.m. UTC 
This patch removes useless and duplicate statements.
Also add a simple err mesg in journal.c to avoid a
value-never-read error.

 *journal.c
  Add return value check.

 *lpt.c & lpt_commit
  No need to set @shft and @done_ltab, value never
  read after these statements.

 *orphan.c
  Remove duplicate code.

 *sb.c & super.c
  The types of @default_compr and @compr_type are
  unsigned int, should never less than zero.

 *tnc.c & tnc_commit.c
  No need to set @err.

Signed-off-by: hujianyang <hujianyang@huawei.com>
---
 fs/ubifs/journal.c    | 3 +++
 fs/ubifs/lpt.c        | 3 ---
 fs/ubifs/lpt_commit.c | 2 --
 fs/ubifs/orphan.c     | 1 -
 fs/ubifs/sb.c         | 2 +-
 fs/ubifs/super.c      | 2 +-
 fs/ubifs/tnc.c        | 1 -
 fs/ubifs/tnc_commit.c | 1 -
 8 files changed, 5 insertions(+), 10 deletions(-)

Comments

Artem Bityutskiy July 1, 2014, 12:37 p.m. UTC | #1 
On Wed, 2014-06-11 at 10:42 +0800, hujianyang wrote:
> diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
> index 0e045e7..f68e088 100644
> --- a/fs/ubifs/journal.c
> +++ b/fs/ubifs/journal.c
> @@ -389,6 +389,9 @@ out:
>  		ubifs_dump_budg(c, &c->bi);
>  		ubifs_dump_lprops(c);
>  		cmt_retries = dbg_check_lprops(c);
> +		if (cmt_retries)
> +			ubifs_err("fail to check lprops, error %d",
> +				  cmt_retries);
>  		up_write(&c->commit_sem);

The general approach we took in UBIFS is that the function prints about
its own problems itself. We tend to avoid not make calling function
print an error message for the called function.

Please, add an error message to 'dbg_check_lprops()' instead.

I've dropped this change. The other ones look OK, so I applied them.

Thanks!
hujianyang July 7, 2014, 8:07 a.m. UTC | #2 
On 2014/7/1 20:37, Artem Bityutskiy wrote:
> On Wed, 2014-06-11 at 10:42 +0800, hujianyang wrote:
>> diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
>> index 0e045e7..f68e088 100644
>> --- a/fs/ubifs/journal.c
>> +++ b/fs/ubifs/journal.c
>> @@ -389,6 +389,9 @@ out:
>>  		ubifs_dump_budg(c, &c->bi);
>>  		ubifs_dump_lprops(c);
>>  		cmt_retries = dbg_check_lprops(c);
>> +		if (cmt_retries)
>> +			ubifs_err("fail to check lprops, error %d",
>> +				  cmt_retries);
>>  		up_write(&c->commit_sem);
> 
> The general approach we took in UBIFS is that the function prints about
> its own problems itself. We tend to avoid not make calling function
> print an error message for the called function.
> 
> Please, add an error message to 'dbg_check_lprops()' instead.

I think error messages in 'dbg_check_lprops' is good enough.

> 
> I've dropped this change. The other ones look OK, so I applied them.
> 
> Thanks!
> 

I add the return value check because a value-never-read warn. We set
this cmt_retries and never read it. But it is really no nead check it
as you said.

I think this is better.

- 		cmt_retries = dbg_check_lprops(c);
+		dbg_check_lprops(c);

There is not need to change this by a separate patch. Just leave it
behind.
diff mbox

Patch

diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index 0e045e7..f68e088 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -389,6 +389,9 @@  out:
 		ubifs_dump_budg(c, &c->bi);
 		ubifs_dump_lprops(c);
 		cmt_retries = dbg_check_lprops(c);
+		if (cmt_retries)
+			ubifs_err("fail to check lprops, error %d",
+				  cmt_retries);
 		up_write(&c->commit_sem);
 	}
 	return err;
diff --git a/fs/ubifs/lpt.c b/fs/ubifs/lpt.c
index b4fb422..421bd0a 100644
--- a/fs/ubifs/lpt.c
+++ b/fs/ubifs/lpt.c
@@ -1464,7 +1464,6 @@  struct ubifs_lprops *ubifs_lpt_lookup(struct ubifs_info *c, int lnum)
 			return ERR_CAST(nnode);
 	}
 	iip = ((i >> shft) & (UBIFS_LPT_FANOUT - 1));
-	shft -= UBIFS_LPT_FANOUT_SHIFT;
 	pnode = ubifs_get_pnode(c, nnode, iip);
 	if (IS_ERR(pnode))
 		return ERR_CAST(pnode);
@@ -1604,7 +1603,6 @@  struct ubifs_lprops *ubifs_lpt_lookup_dirty(struct ubifs_info *c, int lnum)
 			return ERR_CAST(nnode);
 	}
 	iip = ((i >> shft) & (UBIFS_LPT_FANOUT - 1));
-	shft -= UBIFS_LPT_FANOUT_SHIFT;
 	pnode = ubifs_get_pnode(c, nnode, iip);
 	if (IS_ERR(pnode))
 		return ERR_CAST(pnode);
@@ -1964,7 +1962,6 @@  again:
 		}
 	}
 	iip = ((i >> shft) & (UBIFS_LPT_FANOUT - 1));
-	shft -= UBIFS_LPT_FANOUT_SHIFT;
 	pnode = scan_get_pnode(c, path + h, nnode, iip);
 	if (IS_ERR(pnode)) {
 		err = PTR_ERR(pnode);
diff --git a/fs/ubifs/lpt_commit.c b/fs/ubifs/lpt_commit.c
index 7e957b6..db08de4 100644
--- a/fs/ubifs/lpt_commit.c
+++ b/fs/ubifs/lpt_commit.c
@@ -304,7 +304,6 @@  static int layout_cnodes(struct ubifs_info *c)
 			ubifs_assert(lnum >= c->lpt_first &&
 				     lnum <= c->lpt_last);
 		}
-		done_ltab = 1;
 		c->ltab_lnum = lnum;
 		c->ltab_offs = offs;
 		offs += c->ltab_sz;
@@ -514,7 +513,6 @@  static int write_cnodes(struct ubifs_info *c)
 			if (err)
 				return err;
 		}
-		done_ltab = 1;
 		ubifs_pack_ltab(c, buf + offs, c->ltab_cmt);
 		offs += c->ltab_sz;
 		dbg_chk_lpt_sz(c, 1, c->ltab_sz);
diff --git a/fs/ubifs/orphan.c b/fs/ubifs/orphan.c
index f1c3e5a1..4409f48 100644
--- a/fs/ubifs/orphan.c
+++ b/fs/ubifs/orphan.c
@@ -346,7 +346,6 @@  static int write_orph_nodes(struct ubifs_info *c, int atomic)
 		int lnum;

 		/* Unmap any unused LEBs after consolidation */
-		lnum = c->ohead_lnum + 1;
 		for (lnum = c->ohead_lnum + 1; lnum <= c->orph_last; lnum++) {
 			err = ubifs_leb_unmap(c, lnum);
 			if (err)
diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c
index 7ba1378..79c6dbb 100644
--- a/fs/ubifs/sb.c
+++ b/fs/ubifs/sb.c
@@ -449,7 +449,7 @@  static int validate_sb(struct ubifs_info *c, struct ubifs_sb_node *sup)
 		goto failed;
 	}

-	if (c->default_compr < 0 || c->default_compr >= UBIFS_COMPR_TYPES_CNT) {
+	if (c->default_compr >= UBIFS_COMPR_TYPES_CNT) {
 		err = 13;
 		goto failed;
 	}
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 3904c85..3b0c2c0 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -75,7 +75,7 @@  static int validate_inode(struct ubifs_info *c, const struct inode *inode)
 		return 1;
 	}

-	if (ui->compr_type < 0 || ui->compr_type >= UBIFS_COMPR_TYPES_CNT) {
+	if (ui->compr_type >= UBIFS_COMPR_TYPES_CNT) {
 		ubifs_err("unknown compression type %d", ui->compr_type);
 		return 2;
 	}
diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c
index 8a40cf9..6793db0 100644
--- a/fs/ubifs/tnc.c
+++ b/fs/ubifs/tnc.c
@@ -3294,7 +3294,6 @@  int dbg_check_inode_size(struct ubifs_info *c, const struct inode *inode,
 		goto out_unlock;

 	if (err) {
-		err = -EINVAL;
 		key = &from_key;
 		goto out_dump;
 	}
diff --git a/fs/ubifs/tnc_commit.c b/fs/ubifs/tnc_commit.c
index 52a6559..e570734 100644
--- a/fs/ubifs/tnc_commit.c
+++ b/fs/ubifs/tnc_commit.c
@@ -389,7 +389,6 @@  static int layout_in_gaps(struct ubifs_info *c, int cnt)
 				ubifs_dump_lprops(c);
 			}
 			/* Try to commit anyway */
-			err = 0;
 			break;
 		}
 		p++;