[PATCH 1/3] staging: lustre: fix function definition style

[PATCH 1/3] staging: lustre: fix function definition style

[Spencer Baugh]

Fix errors reported by checkpatch of this kind:
ERROR: open brace '{' following function declarations go on the next line

Signed-off-by: Spencer Baugh <sbaugh@andrew.cmu.edu>
---
 drivers/staging/lustre/lustre/include/lustre_import.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_import.h b/drivers/staging/lustre/lustre/include/lustre_import.h
index 8304a55..feb4291 100644
--- a/drivers/staging/lustre/lustre/include/lustre_import.h
+++ b/drivers/staging/lustre/lustre/include/lustre_import.h
@@ -346,21 +346,24 @@ static inline unsigned int at_timeout2est(unsigned int val)
 	return (max((val << 2) / 5, 5U) - 4);
 }
 
-static inline void at_reset(struct adaptive_timeout *at, int val) {
+static inline void at_reset(struct adaptive_timeout *at, int val) 
+{
 	spin_lock(&at->at_lock);
 	at->at_current = val;
 	at->at_worst_ever = val;
 	at->at_worst_time = get_seconds();
 	spin_unlock(&at->at_lock);
 }
-static inline void at_init(struct adaptive_timeout *at, int val, int flags) {
+static inline void at_init(struct adaptive_timeout *at, int val, int flags) 
+{
 	memset(at, 0, sizeof(*at));
 	spin_lock_init(&at->at_lock);
 	at->at_flags = flags;
 	at_reset(at, val);
 }
 extern unsigned int at_min;
-static inline int at_get(struct adaptive_timeout *at) {
+static inline int at_get(struct adaptive_timeout *at) 
+{
 	return (at->at_current > at_min) ? at->at_current : at_min;
 }
 int at_measured(struct adaptive_timeout *at, unsigned int val);
-- 
2.1.0

[PATCH 2/3] staging: lustre: fix pointer whitespace style

[Spencer Baugh]

Fix errors reported by checkpatch of this kind:
ERROR: "foo * bar" should be "foo *bar"

Signed-off-by: Spencer Baugh <sbaugh@andrew.cmu.edu>
---
 drivers/staging/lustre/lustre/include/lustre_import.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_import.h b/drivers/staging/lustre/lustre/include/lustre_import.h
index feb4291..b71877b 100644
--- a/drivers/staging/lustre/lustre/include/lustre_import.h
+++ b/drivers/staging/lustre/lustre/include/lustre_import.h
@@ -103,9 +103,9 @@ enum lustre_imp_state {
 };
 
 /** Returns test string representation of numeric import state \a state */
-static inline char * ptlrpc_import_state_name(enum lustre_imp_state state)
+static inline char *ptlrpc_import_state_name(enum lustre_imp_state state)
 {
-	static char* import_state_names[] = {
+	static char *import_state_names[] = {
 		"<UNKNOWN>", "CLOSED",  "NEW", "DISCONN",
 		"CONNECTING", "REPLAY", "REPLAY_LOCKS", "REPLAY_WAIT",
 		"RECOVER", "FULL", "EVICTED",
-- 
2.1.0

Re: [PATCH 2/3] staging: lustre: fix pointer whitespace style

[Joe Perches]

On Sat, 2014-09-06 at 13:38 -0400, Spencer Baugh wrote:
> Fix errors reported by checkpatch of this kind:
[]
> diff --git a/drivers/staging/lustre/lustre/include/lustre_import.h b/drivers/staging/lustre/lustre/include/lustre_import.h
[]
> @@ -103,9 +103,9 @@ enum lustre_imp_state {
>  };
>  
>  /** Returns test string representation of numeric import state \a state */
> -static inline char * ptlrpc_import_state_name(enum lustre_imp_state state)
> +static inline char *ptlrpc_import_state_name(enum lustre_imp_state state)
>  {
> -	static char* import_state_names[] = {
> +	static char *import_state_names[] = {
>  		"<UNKNOWN>", "CLOSED",  "NEW", "DISCONN",
>  		"CONNECTING", "REPLAY", "REPLAY_LOCKS", "REPLAY_WAIT",
>  		"RECOVER", "FULL", "EVICTED",

This likely shouldn't be static inline, but an actual
function somewhere.

The return should probably be const char *.

Indexing a string array with an enum is generally unsafe.

I think it better to use a switch/case like:

const char *ptlrpc_import_state_name(enum lustre_imp_state state)
{
	switch (state) {
	case LUSTRE_IMP_CLOSED:
		return "CLOSED";
	case LUSTRE_IMP_NEW:
		return "NEW";
	case LUSTRE_IMP_DISCON:
		return "DISCONN";
	case LUSTRE_IMP_CONNECTING:
		return "CONNECTING";
	case LUSTRE_IMP_REPLAY:
		return "REPLAY";
	case LUSTRE_IMP_REPLAY_LOCKS:
		return "REPLAY_LOCKS";
	case LUSTRE_IMP_REPLAY_WAIT:
		return "REPLAY_WAIT";
	case LUSTRE_IMP_RECOVER:
		return "RECOVER";
	case LUSTRE_IMP_FULL:
		return "FULL";
	case LUSTRE_IMP_EVICTED:
		return "EVICTED";
	default:
		return "UNKNOWN";
	}
}

Re: [PATCH 2/3] staging: lustre: fix pointer whitespace style

[Dan Carpenter]

On Sat, Sep 06, 2014 at 11:19:03AM -0700, Joe Perches wrote:
> 
> Indexing a string array with an enum is generally unsafe.
> 
> I think it better to use a switch/case like:
> 

The reverse side of that argument is that switch statements are slower
and uglier.

My understanding is that according to the c spec, "state" could be
signed but in this case GCC defines it as unsigned int.  So the ASSERT
is ok in real life, but in theory it should check for negatives as well.

regards,
dan carpenter

Re: [PATCH 2/3] staging: lustre: fix pointer whitespace style

[Joe Perches]

On Mon, 2014-09-08 at 13:32 +0300, Dan Carpenter wrote:
> On Sat, Sep 06, 2014 at 11:19:03AM -0700, Joe Perches wrote:
> > Indexing a string array with an enum is generally unsafe.
> > I think it better to use a switch/case like:
> The reverse side of that argument is that switch statements are slower
> and uglier.

Uglier is true, safer is true too.
Slower depends.
Sometimes the actual code is identical.

[PATCH 3/3] staging: lustre: remove trailing whitespace

[Spencer Baugh]

Fix errors reported by checkpatch of this kind:
ERROR: trailing whitespace

Signed-off-by: Spencer Baugh <sbaugh@andrew.cmu.edu>
---
 drivers/staging/lustre/lustre/include/lustre_import.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_import.h b/drivers/staging/lustre/lustre/include/lustre_import.h
index b71877b..51f3e98 100644
--- a/drivers/staging/lustre/lustre/include/lustre_import.h
+++ b/drivers/staging/lustre/lustre/include/lustre_import.h
@@ -346,7 +346,7 @@ static inline unsigned int at_timeout2est(unsigned int val)
 	return (max((val << 2) / 5, 5U) - 4);
 }
 
-static inline void at_reset(struct adaptive_timeout *at, int val) 
+static inline void at_reset(struct adaptive_timeout *at, int val)
 {
 	spin_lock(&at->at_lock);
 	at->at_current = val;
@@ -354,7 +354,7 @@ static inline void at_reset(struct adaptive_timeout *at, int val)
 	at->at_worst_time = get_seconds();
 	spin_unlock(&at->at_lock);
 }
-static inline void at_init(struct adaptive_timeout *at, int val, int flags) 
+static inline void at_init(struct adaptive_timeout *at, int val, int flags)
 {
 	memset(at, 0, sizeof(*at));
 	spin_lock_init(&at->at_lock);
@@ -362,7 +362,7 @@ static inline void at_init(struct adaptive_timeout *at, int val, int flags)
 	at_reset(at, val);
 }
 extern unsigned int at_min;
-static inline int at_get(struct adaptive_timeout *at) 
+static inline int at_get(struct adaptive_timeout *at)
 {
 	return (at->at_current > at_min) ? at->at_current : at_min;
 }
-- 
2.1.0

Re: [PATCH 1/3] staging: lustre: fix function definition style

[Greg KH]

On Sat, Sep 06, 2014 at 01:38:27PM -0400, Spencer Baugh wrote:
> Fix errors reported by checkpatch of this kind:
> ERROR: open brace '{' following function declarations go on the next line
> 
> Signed-off-by: Spencer Baugh <sbaugh@andrew.cmu.edu>
> ---
>  drivers/staging/lustre/lustre/include/lustre_import.h | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/include/lustre_import.h b/drivers/staging/lustre/lustre/include/lustre_import.h
> index 8304a55..feb4291 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_import.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_import.h
> @@ -346,21 +346,24 @@ static inline unsigned int at_timeout2est(unsigned int val)
>  	return (max((val << 2) / 5, 5U) - 4);
>  }
>  
> -static inline void at_reset(struct adaptive_timeout *at, int val) {
> +static inline void at_reset(struct adaptive_timeout *at, int val) 

You are adding whitespace errors with this patch, I can't take this
series because of this :(

Please always run your patches through scripts/checkpatch.pl to ensure
you aren't adding new problems.

Can you fix this series of patches up and resend?

thanks,

greg k-h