staging: erofs: Add braces to do-while statements

PATCH In review Thomas Jespersen 5 participants 7 messages Dec 9, 2018 – Dec 10, 2018 Staging

lore

Share & tools

lore

Thomas Jespersen

author

2018-12-09 15:49 UTC #0 lore raw

PATCH 1/1 staging: erofs: Add braces to do-while statements

This fixes warning reported by sparse (with -Wsparse-all).

Signed-off-by Thomas Jespersen <[email protected]>

---
 drivers/staging/erofs/internal.h | 4 ++--
 drivers/staging/erofs/utils.c    | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

drivers/staging/erofs/internal.h +2 -2

index 57575c7f5635..bf180a803446 100644
@@ -232,9 +232,9 @@ static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
 	/* spin if it is temporarily locked at the reclaim path */
 	if (unlikely(o == locked)) {
 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-		do
+		do {
 			cpu_relax();
-		while (atomic_read(&grp->refcount) == locked);
+		} while (atomic_read(&grp->refcount) == locked);
 #endif
 		goto repeat;
 	}

drivers/staging/erofs/utils.c +2 -2

index ea8a962e5c95..cd76df4a48c7 100644
@@ -217,9 +217,9 @@ unsigned long erofs_shrink_scan(struct shrinker *shrink,
 	unsigned long freed = 0;
 
 	spin_lock(&erofs_sb_list_lock);
-	do
+	do {
 		run_no = ++shrinker_run_no;
-	while (run_no == 0);
+	} while (run_no == 0);
 
 	/* Iterate over all mounted superblocks and try to shrink them */
 	p = erofs_sb_list.next;

Signature · 1 line

2.19.2

Greg KH

maintainer

2018-12-09 16:20 UTC #1 lore raw

Quoted 1 line · Thomas Jespersen

This fixes warning reported by sparse (with -Wsparse-all).

Why is sparse warning about this?

Quoted 11 lines

Signed-off-by Thomas Jespersen <laumann.thomas at gmail.com>
---
 drivers/staging/erofs/internal.h | 4 ++--
 drivers/staging/erofs/utils.c    | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/staging/erofs/internal.h +0 -0
index 57575c7f5635..bf180a803446 100644

drivers/staging/erofs/internal.h static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt) · line 237

Quoted 8 lines

drivers/staging/erofs/internal.h +2 -2

@@ -232,9 +232,9 @@ static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
 	/* spin if it is temporarily locked at the reclaim path */
 	if (unlikely(o == locked)) {
 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-		do
+		do {
 			cpu_relax();
-		while (atomic_read(&grp->refcount) == locked);
+		} while (atomic_read(&grp->refcount) == locked);

That looks like valid code to me, why change this?

greg k-h

Joe Perches

2018-12-09 17:27 UTC #2 lore raw

Quoted 4 lines · Greg KH

Quoted 1 line · Thomas Jespersen
This fixes warning reported by sparse (with -Wsparse-all).
Why is sparse warning about this?

Probably because it's the kernel preferred style
to use single statement

	do {
		<foo>;
	} while (<bar>);

over

by about a 20:1 ratio.

Quoted 23 lines

Quoted 11 lines
Signed-off-by Thomas Jespersen <laumann.thomas at gmail.com>obab
---
 drivers/staging/erofs/internal.h | 4 ++--
 drivers/staging/erofs/utils.c    | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/staging/erofs/internal.h +0 -0
index 57575c7f5635..bf180a803446 100644
drivers/staging/erofs/internal.h static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt) · line 237
Quoted 8 lines
drivers/staging/erofs/internal.h +2 -2
@@ -232,9 +232,9 @@ static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
 	/* spin if it is temporarily locked at the reclaim path */
 	if (unlikely(o == locked)) {
 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-		do
+		do {
 			cpu_relax();
-		while (atomic_read(&grp->refcount) == locked);
+		} while (atomic_read(&grp->refcount) == locked);
That looks like valid code to me, why change this?
greg k-h

Greg KH

maintainer

2018-12-09 19:40 UTC #3 lore raw

Quoted 9 lines · Joe Perches

Quoted 4 lines · Greg KH
Quoted 1 line · Thomas Jespersen
This fixes warning reported by sparse (with -Wsparse-all).
Why is sparse warning about this?
Probably because it's the kernel preferred style
to use single statement

Quoted 7 lines

	do {
		<foo>;
	} while (<bar>);
over
by about a 20:1 ratio.

Sparse is spitting out coding style complaints now?

Luc Van Oostenryck

2018-12-10 00:42 UTC #4 lore raw

Quoted 14 lines · Greg KH

Quoted 13 lines · Joe Perches
Quoted 4 lines · Greg KH
Quoted 1 line · Thomas Jespersen
This fixes warning reported by sparse (with -Wsparse-all).
Why is sparse warning about this?
Probably because it's the kernel preferred style
to use single statement
	do {
		<foo>;
	} while (<bar>);

Quoted 5 lines

Quoted 3 lines
over
by about a 20:1 ratio.
Sparse is spitting out coding style complaints now?

To my own surprise, it's effectively sparse that complained:
warning: do-while statement is not a compound statement

with the flag -Wsparse-all (or -Wdo-while).
But -Wsparse-all is not a flag normally used for the kernel (or normaly used on anything at all).

-- Luc

Joe Perches

2018-12-09 17:30 UTC #5 lore raw

In reply to Greg KH

(mrph. premature send)

Quoted 4 lines · Greg KH

Quoted 1 line · Thomas Jespersen
This fixes warning reported by sparse (with -Wsparse-all).
Why is sparse warning about this?

Probably because it's the kernel preferred style
to use single statement

	do {
		<foo>;
	} while (<bar>);

over

	do
		<foo>;
	while (<bar>);

by about a 20:1 ratio.

Gao Xiang

2018-12-09 21:37 UTC #6 lore raw

In reply to Thomas Jespersen

[oops... I found Thomas sent the exactly the same email again.]

Quoted 14 lines · Thomas Jespersen

This fixes warning reported by sparse (with -Wsparse-all).
Signed-off-by Thomas Jespersen <laumann.thomas at gmail.com>
---
 drivers/staging/erofs/internal.h | 4 ++--
 drivers/staging/erofs/utils.c    | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/staging/erofs/internal.h +0 -0
index 57575c7f5635..bf180a803446 100644
@@ -232,9 +232,9 @@ static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
 	/* spin if it is temporarily locked at the reclaim path */

drivers/staging/erofs/internal.h static inline bool erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt) · line 240

Quoted 10 lines

drivers/staging/erofs/internal.h +2 -2

 	if (unlikely(o == locked)) {
 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-		do
+		do {
 			cpu_relax();
-		while (atomic_read(&grp->refcount) == locked);
+		} while (atomic_read(&grp->refcount) == locked);
 #endif
 		goto repeat;
 	}

Could you please check the latest source code? I cannot find the above code...

So I think this patch cannot be applied directly :(

https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/tree/drivers/staging/erofs/internal.h?h=staging-next#n260

... and why you send this patch without modification...

https://lists.ozlabs.org/pipermail/linux-erofs/2018-December/001099.html

https://lists.ozlabs.org/pipermail/linux-erofs/2018-December/001100.html

Thanks,

Gao Xiang