Cleanup for staging lustre obdclass

PATCH Superseded Dafna Hirschfeld 2 participants 11 messages Mar 3, 2018 – Mar 4, 2018 Staging

lore

Share & tools

lore

Dafna Hirschfeld

author

2018-03-03 08:12 UTC #0 lore raw

cover Cleanup for staging lustre obdclass

Fixes of issues found with checkpatch.pl for the lustre obdclass driver.

Dafna Hirschfeld (3):
staging: lustre: obdclass: Fix Comparison issues.
staging: lustre: obdclass: Add 'const' to char* array
staging: lustre: obdclass: Replace 'unsigned' with 'unsigned int'

 drivers/staging/lustre/lustre/obdclass/cl_io.c            |  6 +++---
 drivers/staging/lustre/lustre/obdclass/cl_lock.c          |  4 ++--
 drivers/staging/lustre/lustre/obdclass/cl_object.c        |  2 +-
 drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c |  4 ++--
 drivers/staging/lustre/lustre/obdclass/lprocfs_status.c   |  4 ++--
 drivers/staging/lustre/lustre/obdclass/lu_object.c        | 12 ++++++------
 6 files changed, 16 insertions(+), 16 deletions(-)

Signature · 1 line

2.7.4

Dafna Hirschfeld

author

2018-03-03 08:12 UTC #1 lore raw

PATCH 1/3 staging: lustre: obdclass: Fix Comparison issues.

Replace comparison to NULL with 'not' operator.
Move constants to the right side of a comparison.
Issues found with checkpatch.pl

Signed-off-by Dafna Hirschfeld <[email protected]>

---
 drivers/staging/lustre/lustre/obdclass/cl_io.c            | 6 +++---
 drivers/staging/lustre/lustre/obdclass/cl_lock.c          | 2 +-
 drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c | 4 ++--
 drivers/staging/lustre/lustre/obdclass/lprocfs_status.c   | 2 +-
 drivers/staging/lustre/lustre/obdclass/lu_object.c        | 4 ++--
 5 files changed, 9 insertions(+), 9 deletions(-)

drivers/staging/lustre/lustre/obdclass/cl_io.c +3 -3

index ab84e01..fa4d870 100644
@@ -59,7 +59,7 @@
 
 static inline int cl_io_type_is_valid(enum cl_io_type type)
 {
-	return CIT_READ <= type && type < CIT_OP_NR;
+	return type >= CIT_READ && type < CIT_OP_NR;
 }
 
 static inline int cl_io_is_loopable(const struct cl_io *io)

@@ -72,7 +72,7 @@ static inline int cl_io_is_loopable(const struct cl_io *io)
  */
 int cl_io_is_going(const struct lu_env *env)
 {
-	return cl_env_info(env)->clt_current_io != NULL;
+	return !!cl_env_info(env)->clt_current_io;
 }
 
 /**

@@ -390,7 +390,7 @@ void cl_io_unlock(const struct lu_env *env, struct cl_io *io)
 	const struct cl_io_slice *scan;
 
 	LASSERT(cl_io_is_loopable(io));
-	LASSERT(CIS_IT_STARTED <= io->ci_state && io->ci_state < CIS_UNLOCKED);
+	LASSERT(io->ci_state >= CIS_IT_STARTED && io->ci_state < CIS_UNLOCKED);
 	LINVRNT(cl_io_invariant(io));
 
 	set = &io->ci_lockset;

drivers/staging/lustre/lustre/obdclass/cl_lock.c +1 -1

index 3b683b7..3aae332 100644
@@ -229,7 +229,7 @@ const char *cl_lock_mode_name(const enum cl_lock_mode mode)
 		[CLM_WRITE]   = "W",
 		[CLM_GROUP]   = "G"
 	};
-	if (0 <= mode && mode < ARRAY_SIZE(names))
+	if (mode >= 0 && mode < ARRAY_SIZE(names))
 		return names[mode];
 	else
 		return "U";

drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c +2 -2

index c83b7d7..46322d2 100644
@@ -52,7 +52,7 @@ void lprocfs_counter_add(struct lprocfs_stats *stats, int idx, long amount)
 	if (!stats)
 		return;
 
-	LASSERTF(0 <= idx && idx < stats->ls_num,
+	LASSERTF(idx >= 0 && idx < stats->ls_num,
 		 "idx %d, ls_num %hu\n", idx, stats->ls_num);
 
 	/* With per-client stats, statistics are allocated only for

@@ -101,7 +101,7 @@ void lprocfs_counter_sub(struct lprocfs_stats *stats, int idx, long amount)
 	if (!stats)
 		return;
 
-	LASSERTF(0 <= idx && idx < stats->ls_num,
+	LASSERTF(idx >= 0 && idx < stats->ls_num,
 		 "idx %d, ls_num %hu\n", idx, stats->ls_num);
 
 	/* With per-client stats, statistics are allocated only for

drivers/staging/lustre/lustre/obdclass/lprocfs_status.c +1 -1

index e1f4ef2..64cc746 100644
@@ -1585,7 +1585,7 @@ int ldebugfs_seq_create(struct dentry *parent, const char *name,
 	struct dentry *entry;
 
 	/* Disallow secretly (un)writable entries. */
-	LASSERT((seq_fops->write == NULL) == ((mode & 0222) == 0));
+	LASSERT((!seq_fops->write) == ((mode & 0222) == 0));
 
 	entry = debugfs_create_file(name, mode, parent, data, seq_fops);
 	if (IS_ERR_OR_NULL(entry))

drivers/staging/lustre/lustre/obdclass/lu_object.c +2 -2

index cca6881..a97b2fe 100644
@@ -1396,7 +1396,7 @@ static void key_fini(struct lu_context *ctx, int index)
 void lu_context_key_degister(struct lu_context_key *key)
 {
 	LASSERT(atomic_read(&key->lct_used) >= 1);
-	LINVRNT(0 <= key->lct_index && key->lct_index < ARRAY_SIZE(lu_keys));
+	LINVRNT(key->lct_index >= 0 && key->lct_index < ARRAY_SIZE(lu_keys));
 
 	lu_context_key_quiesce(key);

@@ -1516,7 +1516,7 @@ void *lu_context_key_get(const struct lu_context *ctx,
 			 const struct lu_context_key *key)
 {
 	LINVRNT(ctx->lc_state == LCS_ENTERED);
-	LINVRNT(0 <= key->lct_index && key->lct_index < ARRAY_SIZE(lu_keys));
+	LINVRNT(key->lct_index >= 0 && key->lct_index < ARRAY_SIZE(lu_keys));
 	LASSERT(lu_keys[key->lct_index] == key);
 	return ctx->lc_value[key->lct_index];
 }

Signature · 1 line

2.7.4

Dafna Hirschfeld

author

2018-03-03 08:12 UTC #7 lore raw

PATCH 2/3 staging: lustre: obdclass: Add 'const' to char* array

In reply to Dafna Hirschfeld

Replace 'const char*' arrays with 'const char * const' since the values in the arrays are not changed. Issues found with checkpatch.pl

Signed-off-by Dafna Hirschfeld <[email protected]>

---
 drivers/staging/lustre/lustre/obdclass/cl_lock.c   | 2 +-
 drivers/staging/lustre/lustre/obdclass/cl_object.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

drivers/staging/lustre/lustre/obdclass/cl_lock.c +1 -1

index 3aae332..8a73a66 100644
@@ -224,7 +224,7 @@ EXPORT_SYMBOL(cl_lock_release);
 
 const char *cl_lock_mode_name(const enum cl_lock_mode mode)
 {
-	static const char *names[] = {
+	static const char * const names[] = {
 		[CLM_READ]    = "R",
 		[CLM_WRITE]   = "W",
 		[CLM_GROUP]   = "G"

drivers/staging/lustre/lustre/obdclass/cl_object.c +1 -1

index 7b18d77..7809f6a 100644
@@ -495,7 +495,7 @@ static struct cache_stats cl_env_stats = {
 int cl_site_stats_print(const struct cl_site *site, struct seq_file *m)
 {
 	size_t i;
-	static const char *pstate[] = {
+	static const char * const pstate[] = {
 		[CPS_CACHED]  = "c",
 		[CPS_OWNED]   = "o",
 		[CPS_PAGEOUT] = "w",

Signature · 1 line

2.7.4

Dafna Hirschfeld

author

2018-03-03 08:12 UTC #9 lore raw

PATCH 3/3 staging: lustre: obdclass: Replace 'unsigned' with 'unsigned int'

In reply to Dafna Hirschfeld

Replace 'unsigned' with 'unsigned int' to improve readability. Issues found with checkpatch.pl

Signed-off-by Dafna Hirschfeld <[email protected]>

---
 drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 2 +-
 drivers/staging/lustre/lustre/obdclass/lu_object.c      | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

drivers/staging/lustre/lustre/obdclass/lprocfs_status.c +1 -1

index 64cc746..2ed3505 100644
@@ -1467,7 +1467,7 @@ int lprocfs_write_frac_u64_helper(const char __user *buffer,
 {
 	char kernbuf[22], *end, *pbuf;
 	__u64 whole, frac = 0, units;
-	unsigned frac_d = 1;
+	unsigned int frac_d = 1;
 	int sign = 1;
 
 	if (count > (sizeof(kernbuf) - 1))

drivers/staging/lustre/lustre/obdclass/lu_object.c +4 -4

index a97b2fe..649eed6 100644
@@ -1797,10 +1797,10 @@ int lu_env_refill(struct lu_env *env)
 EXPORT_SYMBOL(lu_env_refill);
 
 struct lu_site_stats {
-	unsigned	lss_populated;
-	unsigned	lss_max_search;
-	unsigned	lss_total;
-	unsigned	lss_busy;
+	unsigned int	lss_populated;
+	unsigned int	lss_max_search;
+	unsigned int	lss_total;
+	unsigned int	lss_busy;
 };
 
 static void lu_site_stats_get(struct cfs_hash *hs,

Signature · 1 line

2.7.4

Julia Lawall

2018-03-03 15:09 UTC #2 lore raw

Re: [Outreachy kernel] [PATCH 1/3] staging: lustre: obdclass: Fix Comparison issues.

In reply to Dafna Hirschfeld

Quoted 2 lines · Dafna Hirschfeld

Replace comparison to NULL with 'not' operator.
Move constants to the right side of a comparison.

Personally, I prefer lower_bound <= x && x < upper_bound. I think the designer of checkpatch prefers x >= lower_bound && x < lower_bound. Maybe the Lustre people have a preference.

Quoted 25 lines

Issues found with checkpatch.pl

Signed-off-by Dafna Hirschfeld <[email protected]>

---
 drivers/staging/lustre/lustre/obdclass/cl_io.c            | 6 +++---
 drivers/staging/lustre/lustre/obdclass/cl_lock.c          | 2 +-
 drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c | 4 ++--
 drivers/staging/lustre/lustre/obdclass/lprocfs_status.c   | 2 +-
 drivers/staging/lustre/lustre/obdclass/lu_object.c        | 4 ++--
 5 files changed, 9 insertions(+), 9 deletions(-)

drivers/staging/lustre/lustre/obdclass/cl_io.c +1 -1

index ab84e01..fa4d870 100644
@@ -59,7 +59,7 @@
 
 static inline int cl_io_type_is_valid(enum cl_io_type type)
 {
-	return CIT_READ <= type && type < CIT_OP_NR;
+	return type >= CIT_READ && type < CIT_OP_NR;
 }
 
 static inline int cl_io_is_loopable(const struct cl_io *io)

drivers/staging/lustre/lustre/obdclass/cl_io.c static inline int cl_io_is_loopable(const struct cl_io *io) · line 75

Quoted 5 lines

drivers/staging/lustre/lustre/obdclass/cl_io.c +1 -1

@@ -72,7 +72,7 @@ static inline int cl_io_is_loopable(const struct cl_io *io)
  */
 int cl_io_is_going(const struct lu_env *env)
 {
-	return cl_env_info(env)->clt_current_io != NULL;
+	return !!cl_env_info(env)->clt_current_io;

It looks like the return type should be bool.

This one is sort of different from the others, and should probably be in a different patch.

julia

Quoted 92 lines

drivers/staging/lustre/lustre/obdclass/cl_io.c +1 -1

 }
 
 /**

@@ -390,7 +390,7 @@ void cl_io_unlock(const struct lu_env *env, struct cl_io *io)
 	const struct cl_io_slice *scan;
 
 	LASSERT(cl_io_is_loopable(io));
-	LASSERT(CIS_IT_STARTED <= io->ci_state && io->ci_state < CIS_UNLOCKED);
+	LASSERT(io->ci_state >= CIS_IT_STARTED && io->ci_state < CIS_UNLOCKED);
 	LINVRNT(cl_io_invariant(io));
 
 	set = &io->ci_lockset;

drivers/staging/lustre/lustre/obdclass/cl_lock.c +1 -1

index 3b683b7..3aae332 100644
@@ -229,7 +229,7 @@ const char *cl_lock_mode_name(const enum cl_lock_mode mode)
 		[CLM_WRITE]   = "W",
 		[CLM_GROUP]   = "G"
 	};
-	if (0 <= mode && mode < ARRAY_SIZE(names))
+	if (mode >= 0 && mode < ARRAY_SIZE(names))
 		return names[mode];
 	else
 		return "U";

drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c +2 -2

index c83b7d7..46322d2 100644
@@ -52,7 +52,7 @@ void lprocfs_counter_add(struct lprocfs_stats *stats, int idx, long amount)
 	if (!stats)
 		return;
 
-	LASSERTF(0 <= idx && idx < stats->ls_num,
+	LASSERTF(idx >= 0 && idx < stats->ls_num,
 		 "idx %d, ls_num %hu\n", idx, stats->ls_num);
 
 	/* With per-client stats, statistics are allocated only for

@@ -101,7 +101,7 @@ void lprocfs_counter_sub(struct lprocfs_stats *stats, int idx, long amount)
 	if (!stats)
 		return;
 
-	LASSERTF(0 <= idx && idx < stats->ls_num,
+	LASSERTF(idx >= 0 && idx < stats->ls_num,
 		 "idx %d, ls_num %hu\n", idx, stats->ls_num);
 
 	/* With per-client stats, statistics are allocated only for

drivers/staging/lustre/lustre/obdclass/lprocfs_status.c +1 -1

index e1f4ef2..64cc746 100644
@@ -1585,7 +1585,7 @@ int ldebugfs_seq_create(struct dentry *parent, const char *name,
 	struct dentry *entry;
 
 	/* Disallow secretly (un)writable entries. */
-	LASSERT((seq_fops->write == NULL) == ((mode & 0222) == 0));
+	LASSERT((!seq_fops->write) == ((mode & 0222) == 0));
 
 	entry = debugfs_create_file(name, mode, parent, data, seq_fops);
 	if (IS_ERR_OR_NULL(entry))

drivers/staging/lustre/lustre/obdclass/lu_object.c +2 -3

index cca6881..a97b2fe 100644
@@ -1396,7 +1396,7 @@ static void key_fini(struct lu_context *ctx, int index)
 void lu_context_key_degister(struct lu_context_key *key)
 {
 	LASSERT(atomic_read(&key->lct_used) >= 1);
-	LINVRNT(0 <= key->lct_index && key->lct_index < ARRAY_SIZE(lu_keys));
+	LINVRNT(key->lct_index >= 0 && key->lct_index < ARRAY_SIZE(lu_keys));
 
 	lu_context_key_quiesce(key);

@@ -1516,7 +1516,7 @@ void *lu_context_key_get(const struct lu_context *ctx,
 			 const struct lu_context_key *key)
 {
 	LINVRNT(ctx->lc_state == LCS_ENTERED);
-	LINVRNT(0 <= key->lct_index && key->lct_index < ARRAY_SIZE(lu_keys));
+	LINVRNT(key->lct_index >= 0 && key->lct_index < ARRAY_SIZE(lu_keys));
 	LASSERT(lu_keys[key->lct_index] == key);
 	return ctx->lc_value[key->lct_index];
 }
--

2.7.4

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/d54195335419311b40f159ee1d5c89a625937899.1520014706.git.dafna3%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Julia Lawall

Acked-by on v1 3/3

2018-03-03 15:18 UTC #10 lore raw

Re: [Outreachy kernel] [PATCH 3/3] staging: lustre: obdclass: Replace 'unsigned' with 'unsigned int'

In reply to Dafna Hirschfeld

Quoted 4 lines · Dafna Hirschfeld

Replace 'unsigned' with 'unsigned int' to improve readability. Issues found with checkpatch.pl
Signed-off-by Dafna Hirschfeld <[email protected]>

Acked-by Julia Lawall <[email protected]>

Quoted 47 lines

---
 drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 2 +-
 drivers/staging/lustre/lustre/obdclass/lu_object.c      | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)
drivers/staging/lustre/lustre/obdclass/lprocfs_status.c +1 -1
index 64cc746..2ed3505 100644
@@ -1467,7 +1467,7 @@ int lprocfs_write_frac_u64_helper(const char __user *buffer,
 {
 	char kernbuf[22], *end, *pbuf;
 	__u64 whole, frac = 0, units;
-	unsigned frac_d = 1;
+	unsigned int frac_d = 1;
 	int sign = 1;
 
 	if (count > (sizeof(kernbuf) - 1))
drivers/staging/lustre/lustre/obdclass/lu_object.c +4 -5
index a97b2fe..649eed6 100644
@@ -1797,10 +1797,10 @@ int lu_env_refill(struct lu_env *env)
 EXPORT_SYMBOL(lu_env_refill);
 
 struct lu_site_stats {
-	unsigned	lss_populated;
-	unsigned	lss_max_search;
-	unsigned	lss_total;
-	unsigned	lss_busy;
+	unsigned int	lss_populated;
+	unsigned int	lss_max_search;
+	unsigned int	lss_total;
+	unsigned int	lss_busy;
 };
 
 static void lu_site_stats_get(struct cfs_hash *hs,
--
2.7.4
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/008d96a213ae753dc3f330ca22e09be5471e8ab1.1520014706.git.dafna3%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Julia Lawall

Acked-by on v1 2/3

2018-03-03 15:19 UTC #8 lore raw

Re: [Outreachy kernel] [PATCH 2/3] staging: lustre: obdclass: Add 'const' to char* array

In reply to Dafna Hirschfeld

Quoted 5 lines · Dafna Hirschfeld

Replace 'const char*' arrays with 'const char * const' since the values in the arrays are not changed. Issues found with checkpatch.pl
Signed-off-by Dafna Hirschfeld <[email protected]>

Acked-by Julia Lawall <[email protected]>

Quoted 41 lines

---
 drivers/staging/lustre/lustre/obdclass/cl_lock.c   | 2 +-
 drivers/staging/lustre/lustre/obdclass/cl_object.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/staging/lustre/lustre/obdclass/cl_lock.c +1 -1
index 3aae332..8a73a66 100644
@@ -224,7 +224,7 @@ EXPORT_SYMBOL(cl_lock_release);
 
 const char *cl_lock_mode_name(const enum cl_lock_mode mode)
 {
-	static const char *names[] = {
+	static const char * const names[] = {
 		[CLM_READ]    = "R",
 		[CLM_WRITE]   = "W",
 		[CLM_GROUP]   = "G"
drivers/staging/lustre/lustre/obdclass/cl_object.c +1 -2
index 7b18d77..7809f6a 100644
@@ -495,7 +495,7 @@ static struct cache_stats cl_env_stats = {
 int cl_site_stats_print(const struct cl_site *site, struct seq_file *m)
 {
 	size_t i;
-	static const char *pstate[] = {
+	static const char * const pstate[] = {
 		[CPS_CACHED]  = "c",
 		[CPS_OWNED]   = "o",
 		[CPS_PAGEOUT] = "w",
--
2.7.4
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/dc905c3f60a58a91ccd636cf5d4d9ed0ea334595.1520014706.git.dafna3%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Julia Lawall

2018-03-04 07:07 UTC #4 lore raw

Re: [Outreachy kernel] [PATCH 1/3] staging: lustre: obdclass: Fix Comparison issues.

Quoted 45 lines · Dafna Hirschfeld

Quoted 44 lines · Julia Lawall
Quoted 2 lines · Dafna Hirschfeld
Replace comparison to NULL with 'not' operator.
Move constants to the right side of a comparison.
Personally, I prefer lower_bound <= x && x < upper_bound. I think the designer of checkpatch prefers x >= lower_bound && x < lower_bound. Maybe the Lustre people have a preference.
Quoted 25 lines
Issues found with checkpatch.pl
Signed-off-by Dafna Hirschfeld <[email protected]>
---
 drivers/staging/lustre/lustre/obdclass/cl_io.c            | 6 +++---
 drivers/staging/lustre/lustre/obdclass/cl_lock.c          | 2 +-
 drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c | 4 ++--
 drivers/staging/lustre/lustre/obdclass/lprocfs_status.c   | 2 +-
 drivers/staging/lustre/lustre/obdclass/lu_object.c        | 4 ++--
 5 files changed, 9 insertions(+), 9 deletions(-)
drivers/staging/lustre/lustre/obdclass/cl_io.c +1 -1
index ab84e01..fa4d870 100644
@@ -59,7 +59,7 @@
 
 static inline int cl_io_type_is_valid(enum cl_io_type type)
 {
-     return CIT_READ <= type && type < CIT_OP_NR;
+     return type >= CIT_READ && type < CIT_OP_NR;
 }
 
 static inline int cl_io_is_loopable(const struct cl_io *io)
drivers/staging/lustre/lustre/obdclass/cl_io.c static inline int cl_io_is_loopable(const struct cl_io *io) · line 75
Quoted 5 lines
drivers/staging/lustre/lustre/obdclass/cl_io.c +1 -1
@@ -72,7 +72,7 @@ static inline int cl_io_is_loopable(const struct cl_io *io)
  */
 int cl_io_is_going(const struct lu_env *env)
 {
-     return cl_env_info(env)->clt_current_io != NULL;
+     return !!cl_env_info(env)->clt_current_io;
It looks like the return type should be bool.

Quoted 6 lines

Quoted 5 lines
This one is sort of different from the others, and should probably be in a different patch.
julia
Ok, but it still bool since it is double exclamation marks.

Sorry, I don't get the point. The return type is declared as int.

julia

Quoted 101 lines

Quoted 86 lines

drivers/staging/lustre/lustre/obdclass/cl_io.c +1 -1

 }
 
 /**

@@ -390,7 +390,7 @@ void cl_io_unlock(const struct lu_env *env, struct cl_io *io)
      const struct cl_io_slice *scan;
 
      LASSERT(cl_io_is_loopable(io));
-     LASSERT(CIS_IT_STARTED <= io->ci_state && io->ci_state < CIS_UNLOCKED);
+     LASSERT(io->ci_state >= CIS_IT_STARTED && io->ci_state < CIS_UNLOCKED);
      LINVRNT(cl_io_invariant(io));
 
      set = &io->ci_lockset;

drivers/staging/lustre/lustre/obdclass/cl_lock.c +1 -1

index 3b683b7..3aae332 100644
@@ -229,7 +229,7 @@ const char *cl_lock_mode_name(const enum cl_lock_mode mode)
              [CLM_WRITE]   = "W",
              [CLM_GROUP]   = "G"
      };
-     if (0 <= mode && mode < ARRAY_SIZE(names))
+     if (mode >= 0 && mode < ARRAY_SIZE(names))
              return names[mode];
      else
              return "U";

drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c +2 -2

index c83b7d7..46322d2 100644
@@ -52,7 +52,7 @@ void lprocfs_counter_add(struct lprocfs_stats *stats, int idx, long amount)
      if (!stats)
              return;
 
-     LASSERTF(0 <= idx && idx < stats->ls_num,
+     LASSERTF(idx >= 0 && idx < stats->ls_num,
               "idx %d, ls_num %hu\n", idx, stats->ls_num);
 
      /* With per-client stats, statistics are allocated only for

@@ -101,7 +101,7 @@ void lprocfs_counter_sub(struct lprocfs_stats *stats, int idx, long amount)
      if (!stats)
              return;
 
-     LASSERTF(0 <= idx && idx < stats->ls_num,
+     LASSERTF(idx >= 0 && idx < stats->ls_num,
               "idx %d, ls_num %hu\n", idx, stats->ls_num);
 
      /* With per-client stats, statistics are allocated only for

drivers/staging/lustre/lustre/obdclass/lprocfs_status.c +1 -1

index e1f4ef2..64cc746 100644
@@ -1585,7 +1585,7 @@ int ldebugfs_seq_create(struct dentry *parent, const char *name,
      struct dentry *entry;
 
      /* Disallow secretly (un)writable entries. */
-     LASSERT((seq_fops->write == NULL) == ((mode & 0222) == 0));
+     LASSERT((!seq_fops->write) == ((mode & 0222) == 0));
 
      entry = debugfs_create_file(name, mode, parent, data, seq_fops);
      if (IS_ERR_OR_NULL(entry))

drivers/staging/lustre/lustre/obdclass/lu_object.c +2 -3

index cca6881..a97b2fe 100644
@@ -1396,7 +1396,7 @@ static void key_fini(struct lu_context *ctx, int index)
 void lu_context_key_degister(struct lu_context_key *key)
 {
      LASSERT(atomic_read(&key->lct_used) >= 1);
-     LINVRNT(0 <= key->lct_index && key->lct_index < ARRAY_SIZE(lu_keys));
+     LINVRNT(key->lct_index >= 0 && key->lct_index < ARRAY_SIZE(lu_keys));
 
      lu_context_key_quiesce(key);

@@ -1516,7 +1516,7 @@ void *lu_context_key_get(const struct lu_context *ctx,
                       const struct lu_context_key *key)
 {
      LINVRNT(ctx->lc_state == LCS_ENTERED);
-     LINVRNT(0 <= key->lct_index && key->lct_index < ARRAY_SIZE(lu_keys));
+     LINVRNT(key->lct_index >= 0 && key->lct_index < ARRAY_SIZE(lu_keys));
      LASSERT(lu_keys[key->lct_index] == key);
      return ctx->lc_value[key->lct_index];
 }
--

2.7.4

Quoted 7 lines

Quoted 7 lines
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/d54195335419311b40f159ee1d5c89a625937899.1520014706.git.dafna3%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAJ1myNR9_bau2kzqyXA2P9hB-pdMjkLHj5Qx2on-Ru8uyRZwNA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Dafna Hirschfeld

author

2018-03-04 07:22 UTC #5 lore raw

Re: [Outreachy kernel] [PATCH 1/3] staging: lustre: obdclass: Fix Comparison issues.

Quoted 52 lines · Julia Lawall

Quoted 48 lines · Dafna Hirschfeld
Quoted 47 lines · Julia Lawall
Quoted 2 lines · Dafna Hirschfeld
Replace comparison to NULL with 'not' operator.
Move constants to the right side of a comparison.
Personally, I prefer lower_bound <= x && x < upper_bound. I think the designer of checkpatch prefers x >= lower_bound && x < lower_bound. Maybe the Lustre people have a preference.
Quoted 25 lines
Issues found with checkpatch.pl
Signed-off-by Dafna Hirschfeld <[email protected]>
---
 drivers/staging/lustre/lustre/obdclass/cl_io.c            | 6 +++---
 drivers/staging/lustre/lustre/obdclass/cl_lock.c          | 2 +-
 drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c | 4 ++--
 drivers/staging/lustre/lustre/obdclass/lprocfs_status.c   | 2 +-
 drivers/staging/lustre/lustre/obdclass/lu_object.c        | 4 ++--
 5 files changed, 9 insertions(+), 9 deletions(-)
drivers/staging/lustre/lustre/obdclass/cl_io.c +1 -1
index ab84e01..fa4d870 100644
@@ -59,7 +59,7 @@
 
 static inline int cl_io_type_is_valid(enum cl_io_type type)
 {
-     return CIT_READ <= type && type < CIT_OP_NR;
+     return type >= CIT_READ && type < CIT_OP_NR;
 }
 
 static inline int cl_io_is_loopable(const struct cl_io *io)
drivers/staging/lustre/lustre/obdclass/cl_io.c static inline int cl_io_is_loopable(const struct cl_io *io) · line 75
Quoted 5 lines
drivers/staging/lustre/lustre/obdclass/cl_io.c +1 -1
@@ -72,7 +72,7 @@ static inline int cl_io_is_loopable(const struct cl_io *io)
  */
 int cl_io_is_going(const struct lu_env *env)
 {
-     return cl_env_info(env)->clt_current_io != NULL;
+     return !!cl_env_info(env)->clt_current_io;
It looks like the return type should be bool.
This one is sort of different from the others, and should probably be in a different patch.

Quoted 7 lines

Quoted 3 lines
Quoted 2 lines
julia
Ok, but it still bool since it is double exclamation marks.
Sorry, I don't get the point. The return type is declared as int.
julia

I meant that the actual return type was a bool and is still a bool. There are many functions in other parts of the kernel code that return bool but in the function declaration it declared to return int.

Quoted 102 lines

Quoted 87 lines

drivers/staging/lustre/lustre/obdclass/cl_io.c +1 -1

 }
 
 /**

@@ -390,7 +390,7 @@ void cl_io_unlock(const struct lu_env *env, struct cl_io *io)
      const struct cl_io_slice *scan;
 
      LASSERT(cl_io_is_loopable(io));
-     LASSERT(CIS_IT_STARTED <= io->ci_state && io->ci_state < CIS_UNLOCKED);
+     LASSERT(io->ci_state >= CIS_IT_STARTED && io->ci_state < CIS_UNLOCKED);
      LINVRNT(cl_io_invariant(io));
 
      set = &io->ci_lockset;

drivers/staging/lustre/lustre/obdclass/cl_lock.c +1 -1

index 3b683b7..3aae332 100644
@@ -229,7 +229,7 @@ const char *cl_lock_mode_name(const enum cl_lock_mode mode)
              [CLM_WRITE]   = "W",
              [CLM_GROUP]   = "G"
      };
-     if (0 <= mode && mode < ARRAY_SIZE(names))
+     if (mode >= 0 && mode < ARRAY_SIZE(names))
              return names[mode];
      else
              return "U";

drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c +2 -2

index c83b7d7..46322d2 100644
@@ -52,7 +52,7 @@ void lprocfs_counter_add(struct lprocfs_stats *stats, int idx, long amount)
      if (!stats)
              return;
 
-     LASSERTF(0 <= idx && idx < stats->ls_num,
+     LASSERTF(idx >= 0 && idx < stats->ls_num,
               "idx %d, ls_num %hu\n", idx, stats->ls_num);
 
      /* With per-client stats, statistics are allocated only for

@@ -101,7 +101,7 @@ void lprocfs_counter_sub(struct lprocfs_stats *stats, int idx, long amount)
      if (!stats)
              return;
 
-     LASSERTF(0 <= idx && idx < stats->ls_num,
+     LASSERTF(idx >= 0 && idx < stats->ls_num,
               "idx %d, ls_num %hu\n", idx, stats->ls_num);
 
      /* With per-client stats, statistics are allocated only for

drivers/staging/lustre/lustre/obdclass/lprocfs_status.c +1 -1

index e1f4ef2..64cc746 100644
@@ -1585,7 +1585,7 @@ int ldebugfs_seq_create(struct dentry *parent, const char *name,
      struct dentry *entry;
 
      /* Disallow secretly (un)writable entries. */
-     LASSERT((seq_fops->write == NULL) == ((mode & 0222) == 0));
+     LASSERT((!seq_fops->write) == ((mode & 0222) == 0));
 
      entry = debugfs_create_file(name, mode, parent, data, seq_fops);
      if (IS_ERR_OR_NULL(entry))

drivers/staging/lustre/lustre/obdclass/lu_object.c +2 -3

index cca6881..a97b2fe 100644
@@ -1396,7 +1396,7 @@ static void key_fini(struct lu_context *ctx, int index)
 void lu_context_key_degister(struct lu_context_key *key)
 {
      LASSERT(atomic_read(&key->lct_used) >= 1);
-     LINVRNT(0 <= key->lct_index && key->lct_index < ARRAY_SIZE(lu_keys));
+     LINVRNT(key->lct_index >= 0 && key->lct_index < ARRAY_SIZE(lu_keys));
 
      lu_context_key_quiesce(key);

@@ -1516,7 +1516,7 @@ void *lu_context_key_get(const struct lu_context *ctx,
                       const struct lu_context_key *key)
 {
      LINVRNT(ctx->lc_state == LCS_ENTERED);
-     LINVRNT(0 <= key->lct_index && key->lct_index < ARRAY_SIZE(lu_keys));
+     LINVRNT(key->lct_index >= 0 && key->lct_index < ARRAY_SIZE(lu_keys));
      LASSERT(lu_keys[key->lct_index] == key);
      return ctx->lc_value[key->lct_index];
 }
--

2.7.4

Quoted 7 lines

Quoted 7 lines
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/d54195335419311b40f159ee1d5c89a625937899.1520014706.git.dafna3%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAJ1myNR9_bau2kzqyXA2P9hB-pdMjkLHj5Qx2on-Ru8uyRZwNA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Julia Lawall

2018-03-04 11:40 UTC #6 lore raw

Re: [Outreachy kernel] [PATCH 1/3] staging: lustre: obdclass: Fix Comparison issues.

Quoted 57 lines · Dafna Hirschfeld

Quoted 56 lines · Julia Lawall
Quoted 51 lines · Dafna Hirschfeld
Quoted 44 lines · Julia Lawall
Quoted 2 lines · Dafna Hirschfeld
Replace comparison to NULL with 'not' operator.
Move constants to the right side of a comparison.
Personally, I prefer lower_bound <= x && x < upper_bound. I think the designer of checkpatch prefers x >= lower_bound && x < lower_bound. Maybe the Lustre people have a preference.
Quoted 25 lines
Issues found with checkpatch.pl
Signed-off-by Dafna Hirschfeld <[email protected]>
---
 drivers/staging/lustre/lustre/obdclass/cl_io.c            | 6 +++---
 drivers/staging/lustre/lustre/obdclass/cl_lock.c          | 2 +-
 drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c | 4 ++--
 drivers/staging/lustre/lustre/obdclass/lprocfs_status.c   | 2 +-
 drivers/staging/lustre/lustre/obdclass/lu_object.c        | 4 ++--
 5 files changed, 9 insertions(+), 9 deletions(-)
drivers/staging/lustre/lustre/obdclass/cl_io.c +1 -1
index ab84e01..fa4d870 100644
@@ -59,7 +59,7 @@
 
 static inline int cl_io_type_is_valid(enum cl_io_type type)
 {
-     return CIT_READ <= type && type < CIT_OP_NR;
+     return type >= CIT_READ && type < CIT_OP_NR;
 }
 
 static inline int cl_io_is_loopable(const struct cl_io *io)
drivers/staging/lustre/lustre/obdclass/cl_io.c static inline int cl_io_is_loopable(const struct cl_io *io) · line 75
Quoted 5 lines
drivers/staging/lustre/lustre/obdclass/cl_io.c +1 -1
@@ -72,7 +72,7 @@ static inline int cl_io_is_loopable(const struct cl_io *io)
  */
 int cl_io_is_going(const struct lu_env *env)
 {
-     return cl_env_info(env)->clt_current_io != NULL;
+     return !!cl_env_info(env)->clt_current_io;
It looks like the return type should be bool.
Quoted 5 lines
This one is sort of different from the others, and should probably be in a different patch.
julia
Ok, but it still bool since it is double exclamation marks.

Quoted 7 lines

Quoted 3 lines
Sorry, I don't get the point. The return type is declared as int.
julia
I meant that the actual return type was a bool and is still a bool. There are many functions in other parts of the kernel code that return bool but in the function declaration it declared to return int.

There are also many functions that have a bool return type. It makes more apparent what is going on.

julia

Quoted 104 lines

Quoted 103 lines

Quoted 88 lines

drivers/staging/lustre/lustre/obdclass/cl_io.c +1 -1

 }
 
 /**

@@ -390,7 +390,7 @@ void cl_io_unlock(const struct lu_env *env, struct cl_io *io)
      const struct cl_io_slice *scan;
 
      LASSERT(cl_io_is_loopable(io));
-     LASSERT(CIS_IT_STARTED <= io->ci_state && io->ci_state < CIS_UNLOCKED);
+     LASSERT(io->ci_state >= CIS_IT_STARTED && io->ci_state < CIS_UNLOCKED);
      LINVRNT(cl_io_invariant(io));
 
      set = &io->ci_lockset;

drivers/staging/lustre/lustre/obdclass/cl_lock.c +1 -1

index 3b683b7..3aae332 100644
@@ -229,7 +229,7 @@ const char *cl_lock_mode_name(const enum cl_lock_mode mode)
              [CLM_WRITE]   = "W",
              [CLM_GROUP]   = "G"
      };
-     if (0 <= mode && mode < ARRAY_SIZE(names))
+     if (mode >= 0 && mode < ARRAY_SIZE(names))
              return names[mode];
      else
              return "U";

drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c +2 -2

index c83b7d7..46322d2 100644
@@ -52,7 +52,7 @@ void lprocfs_counter_add(struct lprocfs_stats *stats, int idx, long amount)
      if (!stats)
              return;
 
-     LASSERTF(0 <= idx && idx < stats->ls_num,
+     LASSERTF(idx >= 0 && idx < stats->ls_num,
               "idx %d, ls_num %hu\n", idx, stats->ls_num);
 
      /* With per-client stats, statistics are allocated only for

@@ -101,7 +101,7 @@ void lprocfs_counter_sub(struct lprocfs_stats *stats, int idx, long amount)
      if (!stats)
              return;
 
-     LASSERTF(0 <= idx && idx < stats->ls_num,
+     LASSERTF(idx >= 0 && idx < stats->ls_num,
               "idx %d, ls_num %hu\n", idx, stats->ls_num);
 
      /* With per-client stats, statistics are allocated only for

drivers/staging/lustre/lustre/obdclass/lprocfs_status.c +1 -1

index e1f4ef2..64cc746 100644
@@ -1585,7 +1585,7 @@ int ldebugfs_seq_create(struct dentry *parent, const char *name,
      struct dentry *entry;
 
      /* Disallow secretly (un)writable entries. */
-     LASSERT((seq_fops->write == NULL) == ((mode & 0222) == 0));
+     LASSERT((!seq_fops->write) == ((mode & 0222) == 0));
 
      entry = debugfs_create_file(name, mode, parent, data, seq_fops);
      if (IS_ERR_OR_NULL(entry))

drivers/staging/lustre/lustre/obdclass/lu_object.c +2 -3

index cca6881..a97b2fe 100644
@@ -1396,7 +1396,7 @@ static void key_fini(struct lu_context *ctx, int index)
 void lu_context_key_degister(struct lu_context_key *key)
 {
      LASSERT(atomic_read(&key->lct_used) >= 1);
-     LINVRNT(0 <= key->lct_index && key->lct_index < ARRAY_SIZE(lu_keys));
+     LINVRNT(key->lct_index >= 0 && key->lct_index < ARRAY_SIZE(lu_keys));
 
      lu_context_key_quiesce(key);

@@ -1516,7 +1516,7 @@ void *lu_context_key_get(const struct lu_context *ctx,
                       const struct lu_context_key *key)
 {
      LINVRNT(ctx->lc_state == LCS_ENTERED);
-     LINVRNT(0 <= key->lct_index && key->lct_index < ARRAY_SIZE(lu_keys));
+     LINVRNT(key->lct_index >= 0 && key->lct_index < ARRAY_SIZE(lu_keys));
      LASSERT(lu_keys[key->lct_index] == key);
      return ctx->lc_value[key->lct_index];
 }
--

2.7.4

Quoted 7 lines

Quoted 7 lines
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/d54195335419311b40f159ee1d5c89a625937899.1520014706.git.dafna3%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAJ1myNR9_bau2kzqyXA2P9hB-pdMjkLHj5Qx2on-Ru8uyRZwNA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.