linux-kernel - Re: [PATCH -next] genirq: Change can_request

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87wna6kvv5.wl-maz@kernel.org>
Date:   Wed, 14 Sep 2022 13:01:50 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Zhang Jianhua <chris.zjh@...wei.com>
Cc:     <tglx@...utronix.de>, <samuel@...lland.org>, <brgl@...ev.pl>,
        <mark.rutland@....com>, <lvjianmin@...ngson.cn>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH -next] genirq: Change can_request_irq() return value type to bool

On Wed, 14 Sep 2022 12:06:15 +0100,
Zhang Jianhua <chris.zjh@...wei.com> wrote:
> 
> The function can_request_irq() is used to judge whether the irq can be
> allocated, so bool type would be more suitable for it.
> 
> Signed-off-by: Zhang Jianhua <chris.zjh@...wei.com>
> ---
>  include/linux/irq.h | 2 +-
>  kernel/irq/manage.c | 8 ++++----
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index c3eb89606c2b..3a60c2313fb9 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -707,7 +707,7 @@ extern void note_interrupt(struct irq_desc *desc, irqreturn_t action_ret);
>  extern int noirqdebug_setup(char *str);
>  
>  /* Checks whether the interrupt can be requested by request_irq(): */
> -extern int can_request_irq(unsigned int irq, unsigned long irqflags);
> +extern bool can_request_irq(unsigned int irq, unsigned long irqflags);
>  
>  /* Dummy irq-chip implementations: */
>  extern struct irq_chip no_irq_chip;
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 40fe7806cc8c..d6940d15bf56 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -925,19 +925,19 @@ EXPORT_SYMBOL(irq_set_irq_wake);
>   * particular irq has been exclusively allocated or is available
>   * for driver use.
>   */
> -int can_request_irq(unsigned int irq, unsigned long irqflags)
> +bool can_request_irq(unsigned int irq, unsigned long irqflags)
>  {
>  	unsigned long flags;
>  	struct irq_desc *desc = irq_get_desc_lock(irq, &flags, 0);
> -	int canrequest = 0;
> +	bool canrequest = false;
>  
>  	if (!desc)
> -		return 0;
> +		return false;
>  
>  	if (irq_settings_can_request(desc)) {
>  		if (!desc->action ||
>  		    irqflags & desc->action->flags & IRQF_SHARED)
> -			canrequest = 1;
> +			canrequest = true;
>  	}
>  	irq_put_desc_unlock(desc, flags);
>  	return canrequest;

I'm sorry, but this is a very pointless change. Not only this doesn't
change anything for this particular code other than being cosmetic,
but it also doesn't help any of the callers which are still using an
int.

In general, this sort of patch only adds noise, and I'd like to see
less of them.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.