Remove unnecessary functions and clean up the code

PATCH In review Shivani Bhardwaj 2 participants 9 messages Oct 29, 2015 Staging

Share & tools

Shivani Bhardwaj

author

2015-10-29 05:31 UTC #0 lore raw

cover Remove unnecessary functions and clean up the code

This patchset removes an unnecessary wrapper function and replaces its calls with appropriate function. Also, an unnecessary return statement is removed.
After applying this patchset, code becomes cleaner.

Shivani Bhardwaj (2):
Staging: lustre: console: Drop unnecessary wrapper function
Staging: lustre: console: Remove irrelevant return statement

 drivers/staging/lustre/lnet/selftest/console.c | 80 ++++++++++++--------------
 1 file changed, 36 insertions(+), 44 deletions(-)

Signature · 1 line

2.1.0

Shivani Bhardwaj

author

2015-10-29 05:32 UTC #1 lore raw

PATCH 1/2 Staging: lustre: console: Drop unnecessary wrapper function

Remove the function lstcon_group_put() and replace all its calls with the function lstcon_group_decref() because the former function just performs the job of calling the latter.

Signed-off-by Shivani Bhardwaj <[email protected]>

---
 drivers/staging/lustre/lnet/selftest/console.c | 78 ++++++++++++--------------
 1 file changed, 36 insertions(+), 42 deletions(-)

drivers/staging/lustre/lnet/selftest/console.c +36 ~~-42~~

index d315dd4..9748a09 100644
@@ -277,12 +277,6 @@ lstcon_group_find(const char *name, lstcon_group_t **grpp)
 	return -ENOENT;
 }
 
-static void
-lstcon_group_put(lstcon_group_t *grp)
-{
-	lstcon_group_decref(grp);
-}
-
 static int
 lstcon_group_ndlink_find(lstcon_group_t *grp, lnet_process_id_t id,
 			 lstcon_ndlink_t **ndlpp, int create)

@@ -436,7 +430,7 @@ lstcon_group_nodes_add(lstcon_group_t *grp,
 	}
 
 	if (rc != 0) {
-		lstcon_group_put(tmp);
+		lstcon_group_decref(tmp);
 		return rc;
 	}

@@ -445,7 +439,7 @@ lstcon_group_nodes_add(lstcon_group_t *grp,
 				     tmp, lstcon_sesrpc_condition, &trans);
 	if (rc != 0) {
 		CERROR("Can't create transaction: %d\n", rc);
-		lstcon_group_put(tmp);
+		lstcon_group_decref(tmp);
 		return rc;
 	}

@@ -460,7 +454,7 @@ lstcon_group_nodes_add(lstcon_group_t *grp,
 	lstcon_rpc_trans_destroy(trans);
 
 	lstcon_group_move(tmp, grp);
-	lstcon_group_put(tmp);
+	lstcon_group_decref(tmp);
 
 	return rc;
 }

@@ -510,12 +504,12 @@ lstcon_group_nodes_remove(lstcon_group_t *grp,
 
 	lstcon_rpc_trans_destroy(trans);
 	/* release nodes anyway, because we can't rollback status */
-	lstcon_group_put(tmp);
+	lstcon_group_decref(tmp);
 
 	return rc;
 error:
 	lstcon_group_move(tmp, grp);
-	lstcon_group_put(tmp);
+	lstcon_group_decref(tmp);
 
 	return rc;
 }

@@ -529,7 +523,7 @@ lstcon_group_add(char *name)
 	rc = (lstcon_group_find(name, &grp) == 0) ? -EEXIST : 0;
 	if (rc != 0) {
 		/* find a group with same name */
-		lstcon_group_put(grp);
+		lstcon_group_decref(grp);
 		return rc;
 	}

@@ -563,14 +557,14 @@ lstcon_nodes_add(char *name, int count, lnet_process_id_t *ids_up,
 	if (grp->grp_ref > 2) {
 		/* referred by other threads or test */
 		CDEBUG(D_NET, "Group %s is busy\n", name);
-		lstcon_group_put(grp);
+		lstcon_group_decref(grp);
 
 		return -EBUSY;
 	}
 
 	rc = lstcon_group_nodes_add(grp, count, ids_up, featp, result_up);
 
-	lstcon_group_put(grp);
+	lstcon_group_decref(grp);
 
 	return rc;
 }

@@ -591,7 +585,7 @@ lstcon_group_del(char *name)
 	if (grp->grp_ref > 2) {
 		/* referred by others threads or test */
 		CDEBUG(D_NET, "Group %s is busy\n", name);
-		lstcon_group_put(grp);
+		lstcon_group_decref(grp);
 		return -EBUSY;
 	}

@@ -600,7 +594,7 @@ lstcon_group_del(char *name)
 				     grp, lstcon_sesrpc_condition, &trans);
 	if (rc != 0) {
 		CERROR("Can't create transaction: %d\n", rc);
-		lstcon_group_put(grp);
+		lstcon_group_decref(grp);
 		return rc;
 	}

@@ -608,10 +602,10 @@ lstcon_group_del(char *name)
 
 	lstcon_rpc_trans_destroy(trans);
 
-	lstcon_group_put(grp);
+	lstcon_group_decref(grp);
 	/* -ref for session, it's destroyed,
 	 * status can't be rolled back, destroy group anyway */
-	lstcon_group_put(grp);
+	lstcon_group_decref(grp);
 
 	return rc;
 }

@@ -631,7 +625,7 @@ lstcon_group_clean(char *name, int args)
 	if (grp->grp_ref > 2) {
 		/* referred by test */
 		CDEBUG(D_NET, "Group %s is busy\n", name);
-		lstcon_group_put(grp);
+		lstcon_group_decref(grp);
 		return -EBUSY;
 	}

@@ -640,10 +634,10 @@ lstcon_group_clean(char *name, int args)
 
 	lstcon_group_drain(grp, args);
 
-	lstcon_group_put(grp);
+	lstcon_group_decref(grp);
 	/* release empty group */
 	if (list_empty(&grp->grp_ndl_list))
-		lstcon_group_put(grp);
+		lstcon_group_decref(grp);
 
 	return 0;
 }

@@ -664,16 +658,16 @@ lstcon_nodes_remove(char *name, int count,
 	if (grp->grp_ref > 2) {
 		/* referred by test */
 		CDEBUG(D_NET, "Group %s is busy\n", name);
-		lstcon_group_put(grp);
+		lstcon_group_decref(grp);
 		return -EBUSY;
 	}
 
 	rc = lstcon_group_nodes_remove(grp, count, ids_up, result_up);
 
-	lstcon_group_put(grp);
+	lstcon_group_decref(grp);
 	/* release empty group */
 	if (list_empty(&grp->grp_ndl_list))
-		lstcon_group_put(grp);
+		lstcon_group_decref(grp);
 
 	return rc;
 }

@@ -694,7 +688,7 @@ lstcon_group_refresh(char *name, struct list_head *result_up)
 	if (grp->grp_ref > 2) {
 		/* referred by test */
 		CDEBUG(D_NET, "Group %s is busy\n", name);
-		lstcon_group_put(grp);
+		lstcon_group_decref(grp);
 		return -EBUSY;
 	}

@@ -705,7 +699,7 @@ lstcon_group_refresh(char *name, struct list_head *result_up)
 	if (rc != 0) {
 		/* local error, return */
 		CDEBUG(D_NET, "Can't create transaction: %d\n", rc);
-		lstcon_group_put(grp);
+		lstcon_group_decref(grp);
 		return rc;
 	}

@@ -715,7 +709,7 @@ lstcon_group_refresh(char *name, struct list_head *result_up)
 
 	lstcon_rpc_trans_destroy(trans);
 	/* -ref for me */
-	lstcon_group_put(grp);
+	lstcon_group_decref(grp);
 
 	return rc;
 }

@@ -797,7 +791,7 @@ lstcon_group_info(char *name, lstcon_ndlist_ent_t *gents_p,
 		/* verbose query */
 		rc = lstcon_nodes_getent(&grp->grp_ndl_list,
 					 index_p, count_p, dents_up);
-		lstcon_group_put(grp);
+		lstcon_group_decref(grp);
 
 		return rc;
 	}

@@ -806,7 +800,7 @@ lstcon_group_info(char *name, lstcon_ndlist_ent_t *gents_p,
 	LIBCFS_ALLOC(gentp, sizeof(lstcon_ndlist_ent_t));
 	if (gentp == NULL) {
 		CERROR("Can't allocate ndlist_ent\n");
-		lstcon_group_put(grp);
+		lstcon_group_decref(grp);
 
 		return -ENOMEM;
 	}

@@ -819,7 +813,7 @@ lstcon_group_info(char *name, lstcon_ndlist_ent_t *gents_p,
 
 	LIBCFS_FREE(gentp, sizeof(lstcon_ndlist_ent_t));
 
-	lstcon_group_put(grp);
+	lstcon_group_decref(grp);
 
 	return 0;
 }

@@ -1096,8 +1090,8 @@ lstcon_batch_destroy(lstcon_batch_t *bat)
 
 		list_del(&test->tes_link);
 
-		lstcon_group_put(test->tes_src_grp);
-		lstcon_group_put(test->tes_dst_grp);
+		lstcon_group_decref(test->tes_src_grp);
+		lstcon_group_decref(test->tes_dst_grp);
 
 		LIBCFS_FREE(test, offsetof(lstcon_test_t,
 					   tes_param[test->tes_paramlen]));

@@ -1352,10 +1346,10 @@ out:
 		LIBCFS_FREE(test, offsetof(lstcon_test_t, tes_param[paramlen]));
 
 	if (dst_grp != NULL)
-		lstcon_group_put(dst_grp);
+		lstcon_group_decref(dst_grp);
 
 	if (src_grp != NULL)
-		lstcon_group_put(src_grp);
+		lstcon_group_decref(src_grp);
 
 	return rc;
 }

@@ -1518,7 +1512,7 @@ lstcon_group_stat(char *grp_name, int timeout, struct list_head *result_up)
 
 	rc = lstcon_ndlist_stat(&grp->grp_ndl_list, timeout, result_up);
 
-	lstcon_group_put(grp);
+	lstcon_group_decref(grp);
 
 	return rc;
 }

@@ -1556,13 +1550,13 @@ lstcon_nodes_stat(int count, lnet_process_id_t *ids_up,
 	}
 
 	if (rc != 0) {
-		lstcon_group_put(tmp);
+		lstcon_group_decref(tmp);
 		return rc;
 	}
 
 	rc = lstcon_ndlist_stat(&tmp->grp_ndl_list, timeout, result_up);
 
-	lstcon_group_put(tmp);
+	lstcon_group_decref(tmp);
 
 	return rc;
 }

@@ -1629,7 +1623,7 @@ lstcon_group_debug(int timeout, char *name,
 
 	rc = lstcon_debug_ndlist(&grp->grp_ndl_list, NULL,
 				 timeout, result_up);
-	lstcon_group_put(grp);
+	lstcon_group_decref(grp);
 
 	return rc;
 }

@@ -1666,14 +1660,14 @@ lstcon_nodes_debug(int timeout,
 	}
 
 	if (rc != 0) {
-		lstcon_group_put(grp);
+		lstcon_group_decref(grp);
 		return rc;
 	}
 
 	rc = lstcon_debug_ndlist(&grp->grp_ndl_list, NULL,
 				 timeout, result_up);
 
-	lstcon_group_put(grp);
+	lstcon_group_decref(grp);
 
 	return rc;
 }

@@ -1847,7 +1841,7 @@ lstcon_session_end(void)
 				     lstcon_group_t, grp_link);
 		LASSERT(grp->grp_ref == 1);
 
-		lstcon_group_put(grp);
+		lstcon_group_decref(grp);
 	}
 
 	/* all nodes should be released */

@@ -1966,7 +1960,7 @@ lstcon_acceptor_handle(srpc_server_rpc_t *rpc)
 out:
 	rep->msg_ses_feats = console_session.ses_features;
 	if (grp != NULL)
-		lstcon_group_put(grp);
+		lstcon_group_decref(grp);
 
 	mutex_unlock(&console_session.ses_mutex);

Signature · 1 line

2.1.0

Shivani Bhardwaj

author

2015-10-29 05:32 UTC #2 lore raw

PATCH 2/2 Staging: lustre: console: Remove irrelevant return statement

In reply to Shivani Bhardwaj

Remove return statement from the function lstcon_group_ndlink_move() as its return type is void.
Fix checkpatch WARNING: void function return statements are not generally useful

Signed-off-by Shivani Bhardwaj <[email protected]>

---
 drivers/staging/lustre/lnet/selftest/console.c | 2 --
 1 file changed, 2 deletions(-)

drivers/staging/lustre/lnet/selftest/console.c +0 -2

index 9748a09..b1eceb4 100644
@@ -318,8 +318,6 @@ lstcon_group_ndlink_move(lstcon_group_t *old,
 	list_add_tail(&ndl->ndl_hlink, &new->grp_ndl_hash[idx]);
 	list_add_tail(&ndl->ndl_link, &new->grp_ndl_list);
 	new->grp_nnode++;
-
-	return;
 }
 
 static void

Signature · 1 line

2.1.0

Sudip Mukherjee

2015-10-29 05:48 UTC #3 lore raw

Re: [Outreachy kernel] [PATCH 2/2] Staging: lustre: console: Remove irrelevant return statement

Quoted 6 lines · Shivani Bhardwaj

Remove return statement from the function lstcon_group_ndlink_move() as its return type is void.
Fix checkpatch WARNING: void function return statements are not generally useful
Signed-off-by Shivani Bhardwaj <[email protected]>

checkpatch is complaining about the "Signed-off-by" because of an extra space.

regards
sudip

Shivani Bhardwaj

author

2015-10-29 05:54 UTC #4 lore raw

Re: [Outreachy kernel] [PATCH 2/2] Staging: lustre: console: Remove irrelevant return statement

Quoted 6 lines · Sudip Mukherjee

Quoted 5 lines · Shivani Bhardwaj
Remove return statement from the function lstcon_group_ndlink_move() as its return type is void.
Fix checkpatch WARNING: void function return statements are not generally useful

Quoted 8 lines

Quoted 1 line
Signed-off-by Shivani Bhardwaj <[email protected]>
checkpatch is complaining about the "Signed-off-by" because of an extra space.
regards
sudip

Sending this again. I'm sorry.
Thank you

Sudip Mukherjee

2015-10-29 05:57 UTC #5 lore raw

Re: [Outreachy kernel] [PATCH 2/2] Staging: lustre: console: Remove irrelevant return statement

Quoted 11 lines · Shivani Bhardwaj

Quoted 8 lines · Sudip Mukherjee
Quoted 6 lines · Shivani Bhardwaj
Remove return statement from the function lstcon_group_ndlink_move() as its return type is void.
Fix checkpatch WARNING: void function return statements are not generally useful
Signed-off-by Shivani Bhardwaj <[email protected]>

Quoted 8 lines

Quoted 6 lines
checkpatch is complaining about the "Signed-off-by" because of an extra space.
regards
sudip
Sending this again. I'm sorry.

BTW, are you adding this manually? Let git add these Signed-off-by and you do not need to worry about these anymore.

regards
sudip

Shivani Bhardwaj

author

2015-10-29 05:59 UTC #6 lore raw

Re: [Outreachy kernel] [PATCH 2/2] Staging: lustre: console: Remove irrelevant return statement

Quoted 19 lines · Sudip Mukherjee

Quoted 18 lines · Shivani Bhardwaj
Quoted 14 lines · Sudip Mukherjee
Quoted 6 lines · Shivani Bhardwaj
Remove return statement from the function lstcon_group_ndlink_move() as its return type is void.
Fix checkpatch WARNING: void function return statements are not generally useful
Signed-off-by Shivani Bhardwaj <[email protected]>
checkpatch is complaining about the "Signed-off-by" because of an extra space.
regards
sudip

Quoted 8 lines

Quoted 1 line
Sending this again. I'm sorry.
BTW, are you adding this manually? Let git add these Signed-off-by and you do not need to worry about these anymore.
regards
sudip

I fixed the commit message by git commit --amend. Did I do anything wrong? Thank you

Shivani Bhardwaj

author

2015-10-29 06:00 UTC #7 lore raw

Re: [Outreachy kernel] [PATCH 2/2] Staging: lustre: console: Remove irrelevant return statement

Quoted 29 lines · Shivani Bhardwaj

Quoted 24 lines · Sudip Mukherjee
Quoted 11 lines · Shivani Bhardwaj
Quoted 8 lines · Sudip Mukherjee
Quoted 6 lines · Shivani Bhardwaj
Remove return statement from the function lstcon_group_ndlink_move() as its return type is void.
Fix checkpatch WARNING: void function return statements are not generally useful
Signed-off-by Shivani Bhardwaj <[email protected]>
Quoted 8 lines
Quoted 6 lines
checkpatch is complaining about the "Signed-off-by" because of an extra space.
regards
sudip
Sending this again. I'm sorry.
BTW, are you adding this manually? Let git add these Signed-off-by and you do not need to worry about these anymore.

Quoted 7 lines

Quoted 3 lines
regards
sudip
I fixed the commit message by git commit --amend. Did I do anything wrong? Thank you

And, should I be sending it again?

Sudip Mukherjee

2015-10-29 10:46 UTC #8 lore raw

Re: [Outreachy kernel] [PATCH 2/2] Staging: lustre: console: Remove irrelevant return statement

Quoted 36 lines · Shivani Bhardwaj

Quoted 33 lines · Shivani Bhardwaj
Quoted 27 lines · Sudip Mukherjee
Quoted 11 lines · Shivani Bhardwaj
Quoted 8 lines · Sudip Mukherjee
Quoted 6 lines · Shivani Bhardwaj
Remove return statement from the function lstcon_group_ndlink_move() as its return type is void.
Fix checkpatch WARNING: void function return statements are not generally useful
Signed-off-by Shivani Bhardwaj <[email protected]>
Quoted 8 lines
Quoted 6 lines
checkpatch is complaining about the "Signed-off-by" because of an extra space.
regards
sudip
Sending this again. I'm sorry.
BTW, are you adding this manually? Let git add these Signed-off-by and you do not need to worry about these anymore.
regards
sudip

Quoted 5 lines

Quoted 3 lines
I fixed the commit message by git commit --amend. Did I do anything wrong? Thank you
And, should I be sending it again?

I am seeing that you have already sent it, but I think Greg will be confused by that patch.

regards
sudip