Remove wrapper function and clean up the code

PATCH In review Shivani Bhardwaj 4 participants 10 messages Nov 6, 2015 – Nov 7, 2015 Staging

Share & tools

Shivani Bhardwaj

author

2015-11-06 17:18 UTC #0 lore raw

cover Remove wrapper function and clean up the code

This patchset removes a wrapper function, its prototype and replace its calls in different files with a standard function.
After applying this patch, code becomes cleaner.

Shivani Bhardwaj (3):
Staging: lustre: module: Replace function calls
Staging: lustre: tracefile: Remove wrapper function
Staging: lustre: tracefile: Remove function prototype

 drivers/staging/lustre/lustre/libcfs/module.c    | 4 ++--
 drivers/staging/lustre/lustre/libcfs/tracefile.c | 9 ++-------
 drivers/staging/lustre/lustre/libcfs/tracefile.h | 1 -
 3 files changed, 4 insertions(+), 10 deletions(-)

Signature · 1 line

2.1.0

Shivani Bhardwaj

author

2015-11-06 17:18 UTC #1 lore raw

PATCH 1/3 Staging: lustre: module: Replace function calls

Replace the calls of function cfs_trace_free_string_buffer() with kfree() as the former function is not required.

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

---
 drivers/staging/lustre/lustre/libcfs/module.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/staging/lustre/lustre/libcfs/module.c +2 -2

index 50e8fd2..516a9e7 100644
@@ -392,7 +392,7 @@ static int __proc_dobitmasks(void *data, int write,
 	} else {
 		rc = cfs_trace_copyin_string(tmpstr, tmpstrlen, buffer, nob);
 		if (rc < 0) {
-			cfs_trace_free_string_buffer(tmpstr, tmpstrlen);
+			kfree(tmpstr);
 			return rc;
 		}

@@ -402,7 +402,7 @@ static int __proc_dobitmasks(void *data, int write,
 			*mask |= D_EMERG;
 	}
 
-	cfs_trace_free_string_buffer(tmpstr, tmpstrlen);
+	kfree(tmpstr);
 	return rc;
 }

Signature · 1 line

2.1.0

Shivani Bhardwaj

author

2015-11-06 17:19 UTC #5 lore raw

PATCH 2/3 Staging: lustre: tracefile: Remove wrapper function

In reply to Shivani Bhardwaj

Remove the function cfs_trace_free_string_buffer() as it can be replaced with the standard function kfree().

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

---
 drivers/staging/lustre/lustre/libcfs/tracefile.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

drivers/staging/lustre/lustre/libcfs/tracefile.c +2 -7

index d55dda8..211047f 100644
@@ -817,11 +817,6 @@ int cfs_trace_allocate_string_buffer(char **str, int nob)
 	return 0;
 }
 
-void cfs_trace_free_string_buffer(char *str, int nob)
-{
-	kfree(str);
-}
-
 int cfs_trace_dump_debug_buffer_usrstr(void __user *usr_str, int usr_str_nob)
 {
 	char	 *str;

@@ -842,7 +837,7 @@ int cfs_trace_dump_debug_buffer_usrstr(void __user *usr_str, int usr_str_nob)
 	}
 	rc = cfs_tracefile_dump_all_pages(str);
 out:
-	cfs_trace_free_string_buffer(str, usr_str_nob + 1);
+	kfree(str);
 	return rc;
 }

@@ -898,7 +893,7 @@ int cfs_trace_daemon_command_usrstr(void __user *usr_str, int usr_str_nob)
 	if (rc == 0)
 		rc = cfs_trace_daemon_command(str);
 
-	cfs_trace_free_string_buffer(str, usr_str_nob + 1);
+	kfree(str);
 	return rc;
 }

Signature · 1 line

2.1.0

Shivani Bhardwaj

author

2015-11-06 17:20 UTC #7 lore raw

PATCH 3/3 Staging: lustre: tracefile: Remove function prototype

In reply to Shivani Bhardwaj

Remove the prototype of function cfs_trace_free_string_buffer() as it is no longer needed.

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

---
 drivers/staging/lustre/lustre/libcfs/tracefile.h | 1 -
 1 file changed, 1 deletion(-)

drivers/staging/lustre/lustre/libcfs/tracefile.h +0 -1

index 73d60e0..ba62005 100644
@@ -70,7 +70,6 @@ int cfs_trace_copyin_string(char *knl_buffer, int knl_buffer_nob,
 int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob,
 			     const char *knl_str, char *append);
 int cfs_trace_allocate_string_buffer(char **str, int nob);
-void cfs_trace_free_string_buffer(char *str, int nob);
 int cfs_trace_dump_debug_buffer_usrstr(void __user *usr_str, int usr_str_nob);
 int cfs_trace_daemon_command(char *str);
 int cfs_trace_daemon_command_usrstr(void __user *usr_str, int usr_str_nob);

Signature · 1 line

2.1.0

Simmons, James A.

Acked-by James Simmons on v1 2/3

2015-11-06 22:03 UTC #6 lore raw

RE: [PATCH 2/3] Staging: lustre: tracefile: Remove wrapper function

In reply to Shivani Bhardwaj

Quoted 7 lines

-----Original Message-----
From: devel [mailto:[email protected]] On Behalf Of Shivani Bhardwaj
Sent: Friday, November 06, 2015 12:19 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]
Subject: [PATCH 2/3] Staging: lustre: tracefile: Remove wrapper function

Quoted 7 lines

Remove the function cfs_trace_free_string_buffer() as it can be replaced with the standard function kfree().
Signed-off-by Shivani Bhardwaj <[email protected]>
---
drivers/staging/lustre/lustre/libcfs/tracefile.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)

Acked-by James Simmons <[email protected]>

drivers/staging/lustre/lustre/libcfs/tracefile.c +2 -7

index d55dda8..211047f 100644
@@ -817,11 +817,6 @@ int cfs_trace_allocate_string_buffer(char **str, int nob)
 	return 0;
 }
 
-void cfs_trace_free_string_buffer(char *str, int nob)
-{
-	kfree(str);
-}
-
 int cfs_trace_dump_debug_buffer_usrstr(void __user *usr_str, int usr_str_nob)
 {
 	char	 *str;

@@ -842,7 +837,7 @@ int cfs_trace_dump_debug_buffer_usrstr(void __user *usr_str, int usr_str_nob)
 	}
 	rc = cfs_tracefile_dump_all_pages(str);
 out:
-	cfs_trace_free_string_buffer(str, usr_str_nob + 1);
+	kfree(str);
 	return rc;
 }

@@ -898,7 +893,7 @@ int cfs_trace_daemon_command_usrstr(void __user *usr_str, int usr_str_nob)
 	if (rc == 0)
 		rc = cfs_trace_daemon_command(str);
 
-	cfs_trace_free_string_buffer(str, usr_str_nob + 1);
+	kfree(str);
 	return rc;
 }

Signature · 6 lines

2.1.0

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Simmons, James A.

Acked-by James Simmons on v1 1/3

2015-11-06 22:04 UTC #2 lore raw

RE: [PATCH 1/3] Staging: lustre: module: Replace function calls

In reply to Shivani Bhardwaj

Quoted 6 lines

From: devel [mailto:[email protected]] On Behalf Of Shivani Bhardwaj
Sent: Friday, November 06, 2015 12:18 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]
Subject: [PATCH 1/3] Staging: lustre: module: Replace function calls

Quoted 7 lines

Replace the calls of function cfs_trace_free_string_buffer() with kfree() as the former function is not required.
Signed-off-by Shivani Bhardwaj <[email protected]>
---
drivers/staging/lustre/lustre/libcfs/module.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Acked-by James Simmons <[email protected]>

drivers/staging/lustre/lustre/libcfs/module.c +2 -2

index 50e8fd2..516a9e7 100644
@@ -392,7 +392,7 @@ static int __proc_dobitmasks(void *data, int write,
 	} else {
 		rc = cfs_trace_copyin_string(tmpstr, tmpstrlen, buffer, nob);
 		if (rc < 0) {
-			cfs_trace_free_string_buffer(tmpstr, tmpstrlen);
+			kfree(tmpstr);
 			return rc;
 		}

@@ -402,7 +402,7 @@ static int __proc_dobitmasks(void *data, int write,
 			*mask |= D_EMERG;
 	}
 
-	cfs_trace_free_string_buffer(tmpstr, tmpstrlen);
+	kfree(tmpstr);
 	return rc;
 }

Signature · 6 lines

2.1.0

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Dilger, Andreas

2015-11-07 07:32 UTC #8 lore raw

Re: [PATCH 3/3] Staging: lustre: tracefile: Remove function prototype

In reply to Shivani Bhardwaj

Quoted 2 lines · Shivani Bhardwaj

Remove the prototype of function cfs_trace_free_string_buffer() as it is no longer needed.

These patches would be a lot more useful if the summary contained the name of the function being removed, rather than "remove function prototype" and variations of that. Something like:

staging: lustre: remove cfs_trace_free_string_buffer prototype
staging: lustre: remove ldlm_pool_set_limit wrapper
staging: lustre: remove ldlm_pool_get_limit wrapper

Not sure if that is grounds for rejection of this patch series (I'll leave that up to Dan and Greg), but definitely something for future patches.

Cheers, Andreas

Quoted 16 lines

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

---
drivers/staging/lustre/lustre/libcfs/tracefile.h | 1 -

1 file changed, 1 deletion(-)

drivers/staging/lustre/lustre/libcfs/tracefile.h +0 -1

index 73d60e0..ba62005 100644
@@ -70,7 +70,6 @@ int cfs_trace_copyin_string(char *knl_buffer, int
 knl_buffer_nob,
 int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob,
 			     const char *knl_str, char *append);
 int cfs_trace_allocate_string_buffer(char **str, int nob);
-void cfs_trace_free_string_buffer(char *str, int nob);

Quoted 9 lines

int cfs_trace_dump_debug_buffer_usrstr(void __user *usr_str, int usr_str_nob);
int cfs_trace_daemon_command(char *str);
int cfs_trace_daemon_command_usrstr(void __user *usr_str, int usr_str_nob);
Signature · 1 line
2.1.0

Cheers, Andreas

Signature · 4 lines

Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division

Shivani Bhardwaj

author

2015-11-07 07:50 UTC #9 lore raw

Re: [PATCH 3/3] Staging: lustre: tracefile: Remove function prototype

Quoted 9 lines · Andreas

Quoted 2 lines · Shivani Bhardwaj
Remove the prototype of function cfs_trace_free_string_buffer() as it is no longer needed.
These patches would be a lot more useful if the summary contained the name of the function being removed, rather than "remove function prototype" and variations of that. Something like:

Quoted 9 lines

staging: lustre: remove cfs_trace_free_string_buffer prototype
staging: lustre: remove ldlm_pool_set_limit wrapper
staging: lustre: remove ldlm_pool_get_limit wrapper
Not sure if that is grounds for rejection of this patch series (I'll leave that up to Dan and Greg), but definitely something for future patches.
Cheers, Andreas

I'll definitely take care of this for future patches that I submit.

Thank you
Shivani

Quoted 35 lines

Quoted 16 lines
Signed-off-by Shivani Bhardwaj <[email protected]>
---
drivers/staging/lustre/lustre/libcfs/tracefile.h | 1 -
1 file changed, 1 deletion(-)
drivers/staging/lustre/lustre/libcfs/tracefile.h +0 -1
index 73d60e0..ba62005 100644
@@ -70,7 +70,6 @@ int cfs_trace_copyin_string(char *knl_buffer, int
 knl_buffer_nob,
 int cfs_trace_copyout_string(char __user *usr_buffer, int usr_buffer_nob,
                           const char *knl_str, char *append);
 int cfs_trace_allocate_string_buffer(char **str, int nob);
-void cfs_trace_free_string_buffer(char *str, int nob);
Quoted 9 lines
int cfs_trace_dump_debug_buffer_usrstr(void __user *usr_str, int usr_str_nob);
int cfs_trace_daemon_command(char *str);
int cfs_trace_daemon_command_usrstr(void __user *usr_str, int usr_str_nob);
--
2.1.0
Cheers, Andreas
--
Andreas Dilger
Lustre Software Architect
Intel High Performance Data Division

Dan Carpenter

2015-11-07 11:15 UTC #3 lore raw

Re: [PATCH 1/3] Staging: lustre: module: Replace function calls

In reply to Simmons, James A.

Quoted 21 lines · James A.

Quoted 6 lines
From: devel [mailto:[email protected]] On Behalf Of Shivani Bhardwaj
Sent: Friday, November 06, 2015 12:18 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]
Subject: [PATCH 1/3] Staging: lustre: module: Replace function calls
Quoted 7 lines
Replace the calls of function cfs_trace_free_string_buffer() with kfree() as the former function is not required.
Signed-off-by Shivani Bhardwaj <[email protected]>
---
drivers/staging/lustre/lustre/libcfs/module.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Acked-by James Simmons <[email protected]>
drivers/staging/lustre/lustre/libcfs/module.c +0 -0
index 50e8fd2..516a9e7 100644

drivers/staging/lustre/lustre/libcfs/module.c static int __proc_dobitmasks(void *data, int write, · line 392

Quoted 1 line

drivers/staging/lustre/lustre/libcfs/module.c +0 -0
@@ -392,7 +392,7 @@ static int __proc_dobitmasks(void *data, int write,
 	} else {

Why do your acks include the original diff without the "> " prefix? How are you even managing that trick?

regards,
dan carpenter

Dilger, Andreas

2015-11-07 19:27 UTC #4 lore raw

Re: [PATCH 1/3] Staging: lustre: module: Replace function calls

Quoted 18 lines · Dan Carpenter

Quoted 16 lines · James A.
Quoted 6 lines
From: devel [mailto:[email protected]] On Behalf Of Shivani Bhardwaj
Sent: Friday, November 06, 2015 12:18 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]
Subject: [PATCH 1/3] Staging: lustre: module: Replace function calls
Quoted 7 lines
Replace the calls of function cfs_trace_free_string_buffer() with kfree() as the former function is not required.
Signed-off-by Shivani Bhardwaj <[email protected]>
---
drivers/staging/lustre/lustre/libcfs/module.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Acked-by James Simmons <[email protected]>

Quoted 9 lines

drivers/staging/lustre/lustre/libcfs/module.c static int __proc_dobitmasks(void *data, int write, · line 392
Quoted 6 lines
drivers/staging/lustre/lustre/libcfs/module.c +0 -0
index 50e8fd2..516a9e7 100644
@@ -392,7 +392,7 @@ static int __proc_dobitmasks(void *data, int write,
   } else {
Why do your acks include the original diff without the "> " prefix? How are you even managing that trick?

I was wondering the same, though it does look nicer.

Cheers, Andreas