[PATCH] sched/fair: fix boolreturn.cocci warning

[PATCH] sched/fair: fix boolreturn.cocci warning

[davidcomponentone]

From: Yang Guang <yang.guang5@zte.com.cn>

The coccinelle report
./kernel/sched/fair.c:9726:9-10:
WARNING: return of 0/1 in function 'imbalanced_active_balance'
with return type bool

Return statements in functions returning bool should use true/false
instead of 1/0.

Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: Yang Guang <yang.guang5@zte.com.cn>
Signed-off-by: David Yang <davidcomponentone@gmail.com>
---
 kernel/sched/fair.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 095b0aa378df..354a36c1ab69 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9723,9 +9723,9 @@ imbalanced_active_balance(struct lb_env *env)
 	 */
 	if ((env->migration_type == migrate_task) &&
 	    (sd->nr_balance_failed > sd->cache_nice_tries+2))
-		return 1;
+		return true;
 
-	return 0;
+	return false;
 }
 
 static int need_active_balance(struct lb_env *env)
-- 
2.30.2

Re: [PATCH] sched/fair: fix boolreturn.cocci warning

[Steven Rostedt]

On Tue,  5 Jul 2022 15:38:22 +0800
Jiapeng Chong <jiapeng.chong@linux.alibaba.com> wrote:

> This was found by coccicheck:
> 
> ./kernel/sched/fair.c:9988:9-10: WARNING: return of 0/1 in function 'imbalanced_active_balance' with return type bool.
> 
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>

As your subject says "fix boolreturn.cocci warning". Can you please?

That is, this change has been presented 10 times before, and every time
it is nacked or ignored. The true fix is to fix boolreturn.cocci to not
warn about this in existing code.

Just in case it wasn't clear. NACK to this change.

-- Steve

[PATCH] sched/fair: fix boolreturn.cocci warning

[Jiapeng Chong]

Return statements in functions returning bool should use true/false
instead of 1/0.

This was found by coccicheck:

./kernel/sched/fair.c:9988:9-10: WARNING: return of 0/1 in function 'imbalanced_active_balance' with return type bool.

Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
 kernel/sched/fair.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index ac64b5bb7cc9..814235b5477a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9985,9 +9985,9 @@ imbalanced_active_balance(struct lb_env *env)
 	 */
 	if ((env->migration_type == migrate_task) &&
 	    (sd->nr_balance_failed > sd->cache_nice_tries+2))
-		return 1;
+		return true;
 
-	return 0;
+	return false;
 }
 
 static int need_active_balance(struct lb_env *env)
-- 
2.20.1.7.g153144c

[PATCH] sched/fair: fix boolreturn.cocci warning

[Jiapeng Chong]

Return statements in functions returning bool should use true/false
instead of 1/0.

./kernel/sched/fair.c:9846:9-10: WARNING: return of 0/1 in function
'imbalanced_active_balance' with return type bool.

Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
 kernel/sched/fair.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 8bcc1dbc544c..bba2f9d94e53 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9843,9 +9843,9 @@ imbalanced_active_balance(struct lb_env *env)
 	 */
 	if ((env->migration_type == migrate_task) &&
 	    (sd->nr_balance_failed > sd->cache_nice_tries+2))
-		return 1;
+		return true;
 
-	return 0;
+	return false;
 }
 
 static int need_active_balance(struct lb_env *env)
-- 
2.20.1.7.g153144c

Re: [PATCH] sched/fair: fix boolreturn.cocci warning

[Joe Perches]

On Tue, 2021-12-14 at 10:52 -0500, Steven Rostedt wrote:
> On Tue, 14 Dec 2021 07:24:34 -0800
> Joe Perches <joe@perches.com> wrote:
> 
> > Perhaps it is more a change for consistency than a fix for correctness.
> > 
> > $ git grep -P 'return\s+(?:true|false)' kernel/sched/ | wc -l
> > 209
> 
> As with those other changes, it should be fixed when the function is
> updated. No need to add the churn now.

<shrug> that's the only bool 1/0 return in the subsystem.

Overall there are very few uses of this pattern in the kernel.

There are about 35000 uses of return true/false
and fewer than 50 uses of return 1/0

$ git grep  -P '\breturn\s+(?:true|false)\s*;' -- '*.[ch]' | wc -l
34828

$ cat bool.cocci
@@
typedef bool;
identifier fn;
@@

 bool fn(...)
 {
	<...
*	return \(1\|0\);
	...>
 }

@@
typedef bool;
identifier fn;
expression e;
@@

 bool fn(...)
 {
	<...
*	return e ? 1 : 0;
	...>
 }

$ spatch -sp-file bool.cocci . | grep -P '^(diff|\@|\-[^\-])'
diff -u -p ./arch/arm/mach-omap2/cm2xxx.c /tmp/nothing/arch/arm/mach-omap2/cm2xxx.c
@@ -58,7 +58,6 @@ static bool omap2xxx_cm_is_clkdm_in_hwsu
-	return (v == OMAP24XX_CLKSTCTRL_ENABLE_AUTO) ? 1 : 0;
diff -u -p ./arch/arm/mach-omap2/cm3xxx.c /tmp/nothing/arch/arm/mach-omap2/cm3xxx.c
@@ -47,7 +47,6 @@ static bool omap3xxx_cm_is_clkdm_in_hwsu
-	return (v == OMAP34XX_CLKSTCTRL_ENABLE_AUTO) ? 1 : 0;
diff -u -p ./arch/arm/mach-omap2/powerdomain.c /tmp/nothing/arch/arm/mach-omap2/powerdomain.c
@@ -968,7 +968,6 @@ int pwrdm_disable_hdwr_sar(struct powerd
-	return (pwrdm && pwrdm->flags & PWRDM_HAS_HDWR_SAR) ? 1 : 0;
diff -u -p ./arch/arm/mach-omap2/prm44xx.c /tmp/nothing/arch/arm/mach-omap2/prm44xx.c
@@ -403,7 +403,6 @@ static u32 omap44xx_prm_read_reset_sourc
-	return (omap4_prminst_read_inst_reg(part, inst, idx)) ? 1 : 0;
diff -u -p ./arch/arm/mm/nommu.c /tmp/nothing/arch/arm/mm/nommu.c
@@ -56,7 +56,6 @@ static inline bool security_extensions_e
-	return 0;
diff -u -p ./arch/x86/boot/compressed/kaslr.c /tmp/nothing/arch/x86/boot/compressed/kaslr.c
@@ -668,7 +668,6 @@ static bool process_mem_region(struct me
-	return 0;
diff -u -p ./arch/x86/events/intel/lbr.c /tmp/nothing/arch/x86/events/intel/lbr.c
@@ -914,7 +914,6 @@ static __always_inline int get_lbr_br_ty
-		return 0;
@@ -922,7 +921,6 @@ static __always_inline bool get_lbr_misp
-		return 0;
diff -u -p ./arch/x86/kvm/mmu/mmu.c /tmp/nothing/arch/x86/kvm/mmu/mmu.c
@@ -1482,7 +1482,6 @@ restart:
-		return 0;
@@ -1623,8 +1622,6 @@ static bool kvm_test_age_rmapp(struct kv
-			return 1;
-	return 0;
diff -u -p ./drivers/char/tpm/tpm_ftpm_tee.c /tmp/nothing/drivers/char/tpm/tpm_ftpm_tee.c
@@ -177,7 +177,6 @@ static u8 ftpm_tee_tpm_op_status(struct
-	return 0;
diff -u -p ./drivers/clk/at91/clk-pll.c /tmp/nothing/drivers/clk/at91/clk-pll.c
@@ -49,7 +49,6 @@ static inline bool clk_pll_ready(struct
-	return status & PLL_STATUS_MASK(id) ? 1 : 0;
diff -u -p ./drivers/irqchip/irq-csky-apb-intc.c /tmp/nothing/drivers/irqchip/irq-csky-apb-intc.c
@@ -136,11 +136,9 @@ static inline bool handle_irq_perbit(str
-		return 0;
-	return 1;
diff -u -p ./drivers/mtd/nand/core.c /tmp/nothing/drivers/mtd/nand/core.c
@@ -22,7 +22,6 @@
-		return 0;
diff -u -p ./drivers/net/ethernet/marvell/octeontx2/af/rvu.c /tmp/nothing/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
@@ -1240,9 +1240,7 @@ static bool is_blktype_attached(struct r
-		return pfvf->npalf ? 1 : 0;
-		return pfvf->nixlf ? 1 : 0;
diff -u -p ./drivers/net/wireless/ath/ath5k/dma.c /tmp/nothing/drivers/net/wireless/ath/ath5k/dma.c
@@ -505,7 +505,6 @@ done:
-	return ath5k_hw_reg_read(ah, AR5K_INTPEND) == 1 ? 1 : 0;
diff -u -p ./drivers/net/wireless/mediatek/mt76/mt7921/sdio_mac.c /tmp/nothing/drivers/net/wireless/mediatek/mt76/mt7921/sdio_mac.c
@@ -219,5 +219,4 @@ bool mt7921s_tx_status_data(struct mt76_
-	return 0;
diff -u -p ./drivers/pci/controller/pcie-xilinx.c /tmp/nothing/drivers/pci/controller/pcie-xilinx.c
@@ -122,8 +122,6 @@ static inline void pcie_write(struct xil
-	return (pcie_read(port, XILINX_PCIE_REG_PSCR) &
-		XILINX_PCIE_REG_PSCR_LNKUP) ? 1 : 0;
diff -u -p ./drivers/scsi/elx/libefc_sli/sli4.c /tmp/nothing/drivers/scsi/elx/libefc_sli/sli4.c
@@ -4036,7 +4036,6 @@ sli_fw_ready(struct sli4 *sli4)
-	return (val & SLI4_PORT_STATUS_RDY) ? 1 : 0;
diff -u -p ./fs/nfsd/nfs2acl.c /tmp/nothing/fs/nfsd/nfs2acl.c
@@ -249,34 +249,26 @@ nfsaclsvc_encode_getaclres(struct svc_rq
-		return 0;
-		return 1;
-		return 0;
-		return 0;
-			return 1;
-		return 0;
-		return 0;
-	return 1;
@@ -286,17 +278,13 @@ nfsaclsvc_encode_accessres(struct svc_rq
-		return 0;
-			return 0;
-			return 0;
-	return 1;
diff -u -p ./kernel/locking/rtmutex.c /tmp/nothing/kernel/locking/rtmutex.c
@@ -372,24 +372,19 @@ static __always_inline bool __waiter_les
-		return 1;
-		return 0;
-		return 0;
-			return 1;
-	return 0;
diff -u -p ./kernel/locking/rwbase_rt.c /tmp/nothing/kernel/locking/rwbase_rt.c
@@ -223,10 +223,8 @@ static inline bool __rwbase_write_tryloc
-		return 1;
-	return 0;
diff -u -p ./kernel/sched/fair.c /tmp/nothing/kernel/sched/fair.c
@@ -9777,9 +9777,7 @@ imbalanced_active_balance(struct lb_env
-		return 1;
-	return 0;

Re: [PATCH] sched/fair: fix boolreturn.cocci warning

[Steven Rostedt]

On Tue, 14 Dec 2021 07:24:34 -0800
Joe Perches <joe@perches.com> wrote:

> Perhaps it is more a change for consistency than a fix for correctness.
> 
> $ git grep -P 'return\s+(?:true|false)' kernel/sched/ | wc -l
> 209

As with those other changes, it should be fixed when the function is
updated. No need to add the churn now.

-- Steve

Re: [PATCH] sched/fair: fix boolreturn.cocci warning

[Joe Perches]

On Tue, 2021-12-14 at 09:14 -0500, Steven Rostedt wrote:
> I'll save Peter Zijlstra time from replying to this.
> 
> On Tue, 14 Dec 2021 11:38:45 +0000
> cgel.zte@gmail.com wrote:
> 
> > From: Changcheng Deng <deng.changcheng@zte.com.cn>
> > 
> > ./kernel/sched/fair.c: 9781: 9-10: WARNING: return of 0/1 in function
> > 'imbalanced_active_balance' with return type bool
> > 
> > Return statements in functions returning bool should use true/false
> > instead of 1/0.
> 
> This is a stupid warning. In C "1" is the same as "true" and "0" is the
> same as "false". 
> 
> This is a bogus error message and something not needed to be fixed.
> 
> Please remove these checks from your scripts.
> 
> NACK

Perhaps it is more a change for consistency than a fix for correctness.

$ git grep -P 'return\s+(?:true|false)' kernel/sched/ | wc -l
209

Re: [PATCH] sched/fair: fix boolreturn.cocci warning

[Steven Rostedt]

I'll save Peter Zijlstra time from replying to this.

On Tue, 14 Dec 2021 11:38:45 +0000
cgel.zte@gmail.com wrote:

> From: Changcheng Deng <deng.changcheng@zte.com.cn>
> 
> ./kernel/sched/fair.c: 9781: 9-10: WARNING: return of 0/1 in function
> 'imbalanced_active_balance' with return type bool
> 
> Return statements in functions returning bool should use true/false
> instead of 1/0.

This is a stupid warning. In C "1" is the same as "true" and "0" is the
same as "false". 

This is a bogus error message and something not needed to be fixed.

Please remove these checks from your scripts.

NACK

-- Steve

> 
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: Changcheng Deng <deng.changcheng@zte.com.cn>
> ---
>  kernel/sched/fair.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index ac5e55441cab..0e62fd7ac0e6 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9778,9 +9778,9 @@ imbalanced_active_balance(struct lb_env *env)
>  	 */
>  	if ((env->migration_type == migrate_task) &&
>  	    (sd->nr_balance_failed > sd->cache_nice_tries+2))
> -		return 1;
> +		return true;
>  
> -	return 0;
> +	return false;
>  }
>  
>  static int need_active_balance(struct lb_env *env)

[PATCH] sched/fair: fix boolreturn.cocci warning

[cgel.zte]

From: Changcheng Deng <deng.changcheng@zte.com.cn>

./kernel/sched/fair.c: 9781: 9-10: WARNING: return of 0/1 in function
'imbalanced_active_balance' with return type bool

Return statements in functions returning bool should use true/false
instead of 1/0.

Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: Changcheng Deng <deng.changcheng@zte.com.cn>
---
 kernel/sched/fair.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index ac5e55441cab..0e62fd7ac0e6 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9778,9 +9778,9 @@ imbalanced_active_balance(struct lb_env *env)
 	 */
 	if ((env->migration_type == migrate_task) &&
 	    (sd->nr_balance_failed > sd->cache_nice_tries+2))
-		return 1;
+		return true;
 
-	return 0;
+	return false;
 }
 
 static int need_active_balance(struct lb_env *env)
-- 
2.25.1