linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] Fixed signedness check
@ 2016-12-26 15:43 Guillermo O. Freschi
  2016-12-26 15:43 ` [PATCH 2/2] Style fixes Guillermo O. Freschi
  2017-01-13  1:17 ` [lustre-devel] [PATCH 1/2] Fixed signedness check Dilger, Andreas
  0 siblings, 2 replies; 6+ messages in thread
From: Guillermo O. Freschi @ 2016-12-26 15:43 UTC (permalink / raw)
  To: Oleg Drokin, lustre-devel, linux-kernel; +Cc: Guillermo O. Freschi

Was `unsigned int`, but `enum`s are signed.

Signed-off-by: Guillermo O. Freschi <kedrot@gmail.com>
---
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
index a4a291acb659..f4cbc89b4f24 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -1148,7 +1148,7 @@ static int lock_matches(struct ldlm_lock *lock, struct lock_match_data *data)
 	return INTERVAL_ITER_STOP;
 }
 
-static unsigned int itree_overlap_cb(struct interval_node *in, void *args)
+static enum interval_iter itree_overlap_cb(struct interval_node *in, void *args)
 {
 	struct ldlm_interval *node = to_ldlm_interval(in);
 	struct lock_match_data *data = args;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] Style fixes
  2016-12-26 15:43 [PATCH 1/2] Fixed signedness check Guillermo O. Freschi
@ 2016-12-26 15:43 ` Guillermo O. Freschi
  2017-01-13  1:20   ` [lustre-devel] " Dilger, Andreas
  2017-01-13 20:48   ` [PATCH v2] " Guillermo O. Freschi
  2017-01-13  1:17 ` [lustre-devel] [PATCH 1/2] Fixed signedness check Dilger, Andreas
  1 sibling, 2 replies; 6+ messages in thread
From: Guillermo O. Freschi @ 2016-12-26 15:43 UTC (permalink / raw)
  To: Oleg Drokin, lustre-devel, linux-kernel; +Cc: Guillermo O. Freschi

Missing braces on `if` statement; misaligned parameter.

Signed-off-by: Guillermo O. Freschi <kedrot@gmail.com>
---
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
index f4cbc89b4f24..a23e7ada3891 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -1024,11 +1024,11 @@ void ldlm_grant_lock(struct ldlm_lock *lock, struct list_head *work_list)
 	if (work_list && lock->l_completion_ast)
 		ldlm_add_ast_work_item(lock, NULL, work_list);
 
-	if (res->lr_type == LDLM_PLAIN || res->lr_type == LDLM_IBITS)
+	if (res->lr_type == LDLM_PLAIN || res->lr_type == LDLM_IBITS) {
 		ldlm_grant_lock_with_skiplist(lock);
-	else if (res->lr_type == LDLM_EXTENT)
+	} else if (res->lr_type == LDLM_EXTENT) {
 		ldlm_extent_add_lock(res, lock);
-	else if (res->lr_type == LDLM_FLOCK) {
+	} else if (res->lr_type == LDLM_FLOCK) {
 		/*
 		 * We should not add locks to granted list in the following cases:
 		 * - this is an UNLOCK but not a real lock;
@@ -1040,8 +1040,9 @@ void ldlm_grant_lock(struct ldlm_lock *lock, struct list_head *work_list)
 		    ldlm_is_test_lock(lock) || ldlm_is_flock_deadlock(lock))
 			return;
 		ldlm_resource_add_lock(res, &res->lr_granted, lock);
-	} else
+	} else {
 		LBUG();
+	}
 
 	ldlm_pool_add(&ldlm_res_to_ns(res)->ns_pool, lock);
 }
@@ -1481,7 +1482,8 @@ int ldlm_fill_lvb(struct ldlm_lock *lock, struct req_capsule *pill,
 							lustre_swab_ost_lvb_v1);
 			else
 				lvb = req_capsule_server_sized_swab_get(pill,
-						&RMF_DLM_LVB, size,
+									&RMF_DLM_LVB,
+									size,
 						lustre_swab_ost_lvb_v1);
 			if (unlikely(!lvb)) {
 				LDLM_ERROR(lock, "no LVB");
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [lustre-devel] [PATCH 1/2] Fixed signedness check
  2016-12-26 15:43 [PATCH 1/2] Fixed signedness check Guillermo O. Freschi
  2016-12-26 15:43 ` [PATCH 2/2] Style fixes Guillermo O. Freschi
@ 2017-01-13  1:17 ` Dilger, Andreas
  1 sibling, 0 replies; 6+ messages in thread
From: Dilger, Andreas @ 2017-01-13  1:17 UTC (permalink / raw)
  To: Guillermo O. Freschi; +Cc: Drokin, Oleg, lustre-devel, linux-kernel

On Dec 26, 2016, at 08:43, Guillermo O. Freschi <kedrot@gmail.com> wrote:
> 
> Was `unsigned int`, but `enum`s are signed.

Interesting that GCC didn't complain that the itree_overlap_cb() function
signature didn't match the argument prototype for interval_search():

  typedef enum interval_iter (*interval_callback_t)(struct interval_node *node,
                                                    void *args);

In the end it is a somewhat cosmetic fix, because the only enum values
(1, 2) were unsigned, but good to be consistent.  Thanks for the fix.

> Signed-off-by: Guillermo O. Freschi <kedrot@gmail.com>

Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>

> ---
> drivers/staging/lustre/lustre/ldlm/ldlm_lock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> index a4a291acb659..f4cbc89b4f24 100644
> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> @@ -1148,7 +1148,7 @@ static int lock_matches(struct ldlm_lock *lock, struct lock_match_data *data)
> 	return INTERVAL_ITER_STOP;
> }
> 
> -static unsigned int itree_overlap_cb(struct interval_node *in, void *args)
> +static enum interval_iter itree_overlap_cb(struct interval_node *in, void *args)
> {
> 	struct ldlm_interval *node = to_ldlm_interval(in);
> 	struct lock_match_data *data = args;
> -- 
> 2.11.0
> 
> _______________________________________________
> lustre-devel mailing list
> lustre-devel@lists.lustre.org
> http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [lustre-devel] [PATCH 2/2] Style fixes
  2016-12-26 15:43 ` [PATCH 2/2] Style fixes Guillermo O. Freschi
@ 2017-01-13  1:20   ` Dilger, Andreas
  2017-01-13 20:48   ` [PATCH v2] " Guillermo O. Freschi
  1 sibling, 0 replies; 6+ messages in thread
From: Dilger, Andreas @ 2017-01-13  1:20 UTC (permalink / raw)
  To: Guillermo O. Freschi; +Cc: Drokin, Oleg, lustre-devel, linux-kernel


> On Dec 26, 2016, at 08:43, Guillermo O. Freschi <kedrot@gmail.com> wrote:
> 
> Missing braces on `if` statement; misaligned parameter.
> 
> Signed-off-by: Guillermo O. Freschi <kedrot@gmail.com>
> ---
> drivers/staging/lustre/lustre/ldlm/ldlm_lock.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> index f4cbc89b4f24..a23e7ada3891 100644
> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> @@ -1024,11 +1024,11 @@ void ldlm_grant_lock(struct ldlm_lock *lock, struct list_head *work_list)
> 	if (work_list && lock->l_completion_ast)
> 		ldlm_add_ast_work_item(lock, NULL, work_list);
> 
> -	if (res->lr_type == LDLM_PLAIN || res->lr_type == LDLM_IBITS)
> +	if (res->lr_type == LDLM_PLAIN || res->lr_type == LDLM_IBITS) {
> 		ldlm_grant_lock_with_skiplist(lock);
> -	else if (res->lr_type == LDLM_EXTENT)
> +	} else if (res->lr_type == LDLM_EXTENT) {
> 		ldlm_extent_add_lock(res, lock);
> -	else if (res->lr_type == LDLM_FLOCK) {
> +	} else if (res->lr_type == LDLM_FLOCK) {
> 		/*
> 		 * We should not add locks to granted list in the following cases:
> 		 * - this is an UNLOCK but not a real lock;

This part is fine.

> @@ -1040,8 +1040,9 @@ void ldlm_grant_lock(struct ldlm_lock *lock, struct list_head *work_list)
> 		    ldlm_is_test_lock(lock) || ldlm_is_flock_deadlock(lock))
> 			return;
> 		ldlm_resource_add_lock(res, &res->lr_granted, lock);
> -	} else
> +	} else {
> 		LBUG();
> +	}
> 
> 	ldlm_pool_add(&ldlm_res_to_ns(res)->ns_pool, lock);
> }

Fine.

> @@ -1481,7 +1482,8 @@ int ldlm_fill_lvb(struct ldlm_lock *lock, struct req_capsule *pill,
> 							lustre_swab_ost_lvb_v1);
> 			else
> 				lvb = req_capsule_server_sized_swab_get(pill,
> -						&RMF_DLM_LVB, size,
> +									&RMF_DLM_LVB,
> +									size,
> 						lustre_swab_ost_lvb_v1);

Not so keen on this one, since "lustre_swab_ost_lvb_v1" can't fit after the
'(' of the original function line, there isn't any benefit to aligning the
other two arguments but not that one.  Better to keep the indentation the
same and save one line of whitespace.

Can you please resubmit without this hunk.

Cheers, Andreas

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] Style fixes
  2016-12-26 15:43 ` [PATCH 2/2] Style fixes Guillermo O. Freschi
  2017-01-13  1:20   ` [lustre-devel] " Dilger, Andreas
@ 2017-01-13 20:48   ` Guillermo O. Freschi
  2017-01-16 16:45     ` Oleg Drokin
  1 sibling, 1 reply; 6+ messages in thread
From: Guillermo O. Freschi @ 2017-01-13 20:48 UTC (permalink / raw)
  To: Drokin, Oleg, lustre-devel, linux-kernel; +Cc: Guillermo O. Freschi

Missing braces on `if` statement.

Signed-off-by: Guillermo O. Freschi <kedrot@gmail.com>

Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
---
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
index f4cbc89b4f24..b66bc02646f1 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -1024,11 +1024,11 @@ void ldlm_grant_lock(struct ldlm_lock *lock, struct list_head *work_list)
 	if (work_list && lock->l_completion_ast)
 		ldlm_add_ast_work_item(lock, NULL, work_list);
 
-	if (res->lr_type == LDLM_PLAIN || res->lr_type == LDLM_IBITS)
+	if (res->lr_type == LDLM_PLAIN || res->lr_type == LDLM_IBITS) {
 		ldlm_grant_lock_with_skiplist(lock);
-	else if (res->lr_type == LDLM_EXTENT)
+	} else if (res->lr_type == LDLM_EXTENT) {
 		ldlm_extent_add_lock(res, lock);
-	else if (res->lr_type == LDLM_FLOCK) {
+	} else if (res->lr_type == LDLM_FLOCK) {
 		/*
 		 * We should not add locks to granted list in the following cases:
 		 * - this is an UNLOCK but not a real lock;
@@ -1040,8 +1040,9 @@ void ldlm_grant_lock(struct ldlm_lock *lock, struct list_head *work_list)
 		    ldlm_is_test_lock(lock) || ldlm_is_flock_deadlock(lock))
 			return;
 		ldlm_resource_add_lock(res, &res->lr_granted, lock);
-	} else
+	} else {
 		LBUG();
+	}
 
 	ldlm_pool_add(&ldlm_res_to_ns(res)->ns_pool, lock);
 }
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] Style fixes
  2017-01-13 20:48   ` [PATCH v2] " Guillermo O. Freschi
@ 2017-01-16 16:45     ` Oleg Drokin
  0 siblings, 0 replies; 6+ messages in thread
From: Oleg Drokin @ 2017-01-16 16:45 UTC (permalink / raw)
  To: Guillermo O. Freschi; +Cc: lustre-devel, linux-kernel


On Jan 13, 2017, at 3:48 PM, Guillermo O. Freschi wrote:

> Missing braces on `if` statement.
> 
> Signed-off-by: Guillermo O. Freschi <kedrot@gmail.com>
> 
> Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>

Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
> ---
> drivers/staging/lustre/lustre/ldlm/ldlm_lock.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> index f4cbc89b4f24..b66bc02646f1 100644
> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> @@ -1024,11 +1024,11 @@ void ldlm_grant_lock(struct ldlm_lock *lock, struct list_head *work_list)
> 	if (work_list && lock->l_completion_ast)
> 		ldlm_add_ast_work_item(lock, NULL, work_list);
> 
> -	if (res->lr_type == LDLM_PLAIN || res->lr_type == LDLM_IBITS)
> +	if (res->lr_type == LDLM_PLAIN || res->lr_type == LDLM_IBITS) {
> 		ldlm_grant_lock_with_skiplist(lock);
> -	else if (res->lr_type == LDLM_EXTENT)
> +	} else if (res->lr_type == LDLM_EXTENT) {
> 		ldlm_extent_add_lock(res, lock);
> -	else if (res->lr_type == LDLM_FLOCK) {
> +	} else if (res->lr_type == LDLM_FLOCK) {
> 		/*
> 		 * We should not add locks to granted list in the following cases:
> 		 * - this is an UNLOCK but not a real lock;
> @@ -1040,8 +1040,9 @@ void ldlm_grant_lock(struct ldlm_lock *lock, struct list_head *work_list)
> 		    ldlm_is_test_lock(lock) || ldlm_is_flock_deadlock(lock))
> 			return;
> 		ldlm_resource_add_lock(res, &res->lr_granted, lock);
> -	} else
> +	} else {
> 		LBUG();
> +	}
> 
> 	ldlm_pool_add(&ldlm_res_to_ns(res)->ns_pool, lock);
> }
> -- 
> 2.11.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-01-16 16:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-26 15:43 [PATCH 1/2] Fixed signedness check Guillermo O. Freschi
2016-12-26 15:43 ` [PATCH 2/2] Style fixes Guillermo O. Freschi
2017-01-13  1:20   ` [lustre-devel] " Dilger, Andreas
2017-01-13 20:48   ` [PATCH v2] " Guillermo O. Freschi
2017-01-16 16:45     ` Oleg Drokin
2017-01-13  1:17 ` [lustre-devel] [PATCH 1/2] Fixed signedness check Dilger, Andreas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).