[PATCH 0/3] Cleanup for staging lustre obdclass

[PATCH 0/3] Cleanup for staging lustre obdclass

[Dafna Hirschfeld]

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(-)

-- 
2.7.4

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

[Dafna Hirschfeld]

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 <dafna3@gmail.com>
---
 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(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/cl_io.c b/drivers/staging/lustre/lustre/obdclass/cl_io.c
index ab84e01..fa4d870 100644
--- a/drivers/staging/lustre/lustre/obdclass/cl_io.c
+++ b/drivers/staging/lustre/lustre/obdclass/cl_io.c
@@ -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;
diff --git a/drivers/staging/lustre/lustre/obdclass/cl_lock.c b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
index 3b683b7..3aae332 100644
--- a/drivers/staging/lustre/lustre/obdclass/cl_lock.c
+++ b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
@@ -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";
diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
index c83b7d7..46322d2 100644
--- a/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
+++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
@@ -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
diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
index e1f4ef2..64cc746 100644
--- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
+++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
@@ -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))
diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c b/drivers/staging/lustre/lustre/obdclass/lu_object.c
index cca6881..a97b2fe 100644
--- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
+++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
@@ -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

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

[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 <dafna3@gmail.com>
---
 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(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/cl_lock.c b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
index 3aae332..8a73a66 100644
--- a/drivers/staging/lustre/lustre/obdclass/cl_lock.c
+++ b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
@@ -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"
diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
index 7b18d77..7809f6a 100644
--- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
+++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
@@ -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

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

[Dafna Hirschfeld]

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

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 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(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
index 64cc746..2ed3505 100644
--- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
+++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
@@ -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))
diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c b/drivers/staging/lustre/lustre/obdclass/lu_object.c
index a97b2fe..649eed6 100644
--- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
+++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
@@ -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

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

[Julia Lawall]

On Sat, 3 Mar 2018, Dafna Hirschfeld wrote:

> 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.

> Issues found with checkpatch.pl
>
> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> ---
>  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(-)
>
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_io.c b/drivers/staging/lustre/lustre/obdclass/cl_io.c
> index ab84e01..fa4d870 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_io.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_io.c
> @@ -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;

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

>  }
>
>  /**
> @@ -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;
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_lock.c b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> index 3b683b7..3aae332 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> @@ -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";
> diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
> index c83b7d7..46322d2 100644
> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
> @@ -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
> diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> index e1f4ef2..64cc746 100644
> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> @@ -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))
> diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c b/drivers/staging/lustre/lustre/obdclass/lu_object.c
> index cca6881..a97b2fe 100644
> --- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
> @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>

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

[Julia Lawall]

On Sat, 3 Mar 2018, Dafna Hirschfeld wrote:

> Replace 'unsigned' with 'unsigned int' to improve readability.
> Issues found with checkpatch.pl
>
> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>

Acked-by: Julia Lawall <julia.lawall@lip6.fr>

> ---
>  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(-)
>
> diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> index 64cc746..2ed3505 100644
> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> @@ -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))
> diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c b/drivers/staging/lustre/lustre/obdclass/lu_object.c
> index a97b2fe..649eed6 100644
> --- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
> @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>

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

[Julia Lawall]

On Sat, 3 Mar 2018, Dafna Hirschfeld wrote:

> 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 <dafna3@gmail.com>

Acked-by: Julia Lawall <julia.lawall@lip6.fr>

> ---
>  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(-)
>
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_lock.c b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> index 3aae332..8a73a66 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> @@ -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"
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> index 7b18d77..7809f6a 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>

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

[Dafna Hirschfeld]

On Sat, Mar 3, 2018 at 5:09 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Sat, 3 Mar 2018, Dafna Hirschfeld wrote:
>
>> 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.
>
>> Issues found with checkpatch.pl
>>
>> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
>> ---
>>  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(-)
>>
>> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_io.c b/drivers/staging/lustre/lustre/obdclass/cl_io.c
>> index ab84e01..fa4d870 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/cl_io.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/cl_io.c
>> @@ -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;
>
> 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
>
Ok, but it still bool since it is double exclamation marks.

>>  }
>>
>>  /**
>> @@ -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;
>> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_lock.c b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
>> index 3b683b7..3aae332 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/cl_lock.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
>> @@ -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";
>> diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
>> index c83b7d7..46322d2 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
>> @@ -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
>> diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
>> index e1f4ef2..64cc746 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
>> @@ -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))
>> diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c b/drivers/staging/lustre/lustre/obdclass/lu_object.c
>> index cca6881..a97b2fe 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
>> @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> 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.
>>

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

[Julia Lawall]

On Sun, 4 Mar 2018, Dafna Hirschfeld wrote:

> On Sat, Mar 3, 2018 at 5:09 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >
> >
> > On Sat, 3 Mar 2018, Dafna Hirschfeld wrote:
> >
> >> 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.
> >
> >> Issues found with checkpatch.pl
> >>
> >> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> >> ---
> >>  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(-)
> >>
> >> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_io.c b/drivers/staging/lustre/lustre/obdclass/cl_io.c
> >> index ab84e01..fa4d870 100644
> >> --- a/drivers/staging/lustre/lustre/obdclass/cl_io.c
> >> +++ b/drivers/staging/lustre/lustre/obdclass/cl_io.c
> >> @@ -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;
> >
> > 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
> >
> 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

>
> >>  }
> >>
> >>  /**
> >> @@ -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;
> >> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_lock.c b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> >> index 3b683b7..3aae332 100644
> >> --- a/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> >> +++ b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> >> @@ -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";
> >> diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
> >> index c83b7d7..46322d2 100644
> >> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
> >> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
> >> @@ -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
> >> diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> >> index e1f4ef2..64cc746 100644
> >> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> >> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> >> @@ -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))
> >> diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c b/drivers/staging/lustre/lustre/obdclass/lu_object.c
> >> index cca6881..a97b2fe 100644
> >> --- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
> >> +++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
> >> @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
> >> 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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>

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

[Dafna Hirschfeld]

On Sun, Mar 4, 2018 at 9:07 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Sun, 4 Mar 2018, Dafna Hirschfeld wrote:
>
>> On Sat, Mar 3, 2018 at 5:09 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> >
>> >
>> > On Sat, 3 Mar 2018, Dafna Hirschfeld wrote:
>> >
>> >> 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.
>> >
>> >> Issues found with checkpatch.pl
>> >>
>> >> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
>> >> ---
>> >>  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(-)
>> >>
>> >> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_io.c b/drivers/staging/lustre/lustre/obdclass/cl_io.c
>> >> index ab84e01..fa4d870 100644
>> >> --- a/drivers/staging/lustre/lustre/obdclass/cl_io.c
>> >> +++ b/drivers/staging/lustre/lustre/obdclass/cl_io.c
>> >> @@ -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;
>> >
>> > 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
>> >
>> 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.

>
>>
>> >>  }
>> >>
>> >>  /**
>> >> @@ -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;
>> >> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_lock.c b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
>> >> index 3b683b7..3aae332 100644
>> >> --- a/drivers/staging/lustre/lustre/obdclass/cl_lock.c
>> >> +++ b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
>> >> @@ -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";
>> >> diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
>> >> index c83b7d7..46322d2 100644
>> >> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
>> >> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
>> >> @@ -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
>> >> diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
>> >> index e1f4ef2..64cc746 100644
>> >> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
>> >> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
>> >> @@ -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))
>> >> diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c b/drivers/staging/lustre/lustre/obdclass/lu_object.c
>> >> index cca6881..a97b2fe 100644
>> >> --- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
>> >> +++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
>> >> @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
>> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> >> 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 outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> 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.
>>

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

[Julia Lawall]

On Sun, 4 Mar 2018, Dafna Hirschfeld wrote:

> On Sun, Mar 4, 2018 at 9:07 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >
> >
> > On Sun, 4 Mar 2018, Dafna Hirschfeld wrote:
> >
> >> On Sat, Mar 3, 2018 at 5:09 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >> >
> >> >
> >> > On Sat, 3 Mar 2018, Dafna Hirschfeld wrote:
> >> >
> >> >> 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.
> >> >
> >> >> Issues found with checkpatch.pl
> >> >>
> >> >> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> >> >> ---
> >> >>  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(-)
> >> >>
> >> >> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_io.c b/drivers/staging/lustre/lustre/obdclass/cl_io.c
> >> >> index ab84e01..fa4d870 100644
> >> >> --- a/drivers/staging/lustre/lustre/obdclass/cl_io.c
> >> >> +++ b/drivers/staging/lustre/lustre/obdclass/cl_io.c
> >> >> @@ -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;
> >> >
> >> > 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
> >> >
> >> 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.

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

julia

>
> >
> >>
> >> >>  }
> >> >>
> >> >>  /**
> >> >> @@ -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;
> >> >> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_lock.c b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> >> >> index 3b683b7..3aae332 100644
> >> >> --- a/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> >> >> +++ b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> >> >> @@ -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";
> >> >> diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
> >> >> index c83b7d7..46322d2 100644
> >> >> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
> >> >> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_counters.c
> >> >> @@ -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
> >> >> diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> >> >> index e1f4ef2..64cc746 100644
> >> >> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> >> >> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> >> >> @@ -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))
> >> >> diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c b/drivers/staging/lustre/lustre/obdclass/lu_object.c
> >> >> index cca6881..a97b2fe 100644
> >> >> --- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
> >> >> +++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
> >> >> @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
> >> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
> >> >> 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 outreachy-kernel+unsubscribe@googlegroups.com.
> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
> >> 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.
> >>
>