diff mbox series

drivers/irqchip: Remove function callback casts

Message ID 20200524080910.13087-1-oscar.carter@gmx.com (mailing list archive)
State New, archived
Headers show
Series drivers/irqchip: Remove function callback casts | expand

Commit Message

Oscar Carter May 24, 2020, 8:09 a.m. UTC
In an effort to enable -Wcast-function-type in the top-level Makefile to
support Control Flow Integrity builds, remove all the function callback
casts.

To do this, modify the IRQCHIP_ACPI_DECLARE macro initializing the
acpi_probe_entry struct directly instead of use the existent macro
ACPI_DECLARE_PROBE_ENTRY.

In this new initialization use the probe_subtbl field instead of the
probe_table field use in the ACPI_DECLARE_PROBE_ENTRY macro.

Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
---
 include/linux/irqchip.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

--
2.20.1

Comments

Marc Zyngier May 24, 2020, 11:46 a.m. UTC | #1
On Sun, 24 May 2020 10:09:10 +0200
Oscar Carter <oscar.carter@gmx.com> wrote:

Hi Oscar,

Thanks for this. Comments below.

> In an effort to enable -Wcast-function-type in the top-level Makefile to
> support Control Flow Integrity builds, remove all the function callback
> casts.
> 
> To do this, modify the IRQCHIP_ACPI_DECLARE macro initializing the
> acpi_probe_entry struct directly instead of use the existent macro
> ACPI_DECLARE_PROBE_ENTRY.
> 
> In this new initialization use the probe_subtbl field instead of the
> probe_table field use in the ACPI_DECLARE_PROBE_ENTRY macro.

Please add *why* this is a valid transformation (probe_table and
probe_subtbl are part of a union).

> 
> Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> ---
>  include/linux/irqchip.h | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/irqchip.h b/include/linux/irqchip.h
> index 950e4b2458f0..1f464fd10df0 100644
> --- a/include/linux/irqchip.h
> +++ b/include/linux/irqchip.h
> @@ -39,8 +39,14 @@
>   * @fn: initialization function
>   */
>  #define IRQCHIP_ACPI_DECLARE(name, subtable, validate, data, fn)	\
> -	ACPI_DECLARE_PROBE_ENTRY(irqchip, name, ACPI_SIG_MADT, 		\
> -				 subtable, validate, data, fn)
> +	static const struct acpi_probe_entry __acpi_probe_##name	\
> +		__used __section(__irqchip_acpi_probe_table) = {	\
> +			.id = ACPI_SIG_MADT,				\
> +			.type = subtable,				\
> +			.subtable_valid = validate,			\
> +			.probe_subtbl = (acpi_tbl_entry_handler)fn,	\
> +			.driver_data = data,				\
> +		}
> 

I'd rather you add an ACPI_DECLARE_SUBTABLE_PROBE_ENTRY to acpi.h, and
use that here so that we can keep the ACPI gunk in a single place.

>  #ifdef CONFIG_IRQCHIP
>  void irqchip_init(void);
> --
> 2.20.1
> 
> 

Thanks,

	M.
Oscar Carter May 24, 2020, 4:06 p.m. UTC | #2
Hi Marc,

On Sun, May 24, 2020 at 12:46:34PM +0100, Marc Zyngier wrote:
> On Sun, 24 May 2020 10:09:10 +0200
> Oscar Carter <oscar.carter@gmx.com> wrote:
>
> Hi Oscar,
>
> Thanks for this. Comments below.
>
> > In an effort to enable -Wcast-function-type in the top-level Makefile to
> > support Control Flow Integrity builds, remove all the function callback
> > casts.
> >
> > To do this, modify the IRQCHIP_ACPI_DECLARE macro initializing the
> > acpi_probe_entry struct directly instead of use the existent macro
> > ACPI_DECLARE_PROBE_ENTRY.
> >
> > In this new initialization use the probe_subtbl field instead of the
> > probe_table field use in the ACPI_DECLARE_PROBE_ENTRY macro.
>
> Please add *why* this is a valid transformation (probe_table and
> probe_subtbl are part of a union).

Ok, I will add a more detailed explanation.

> > Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> > ---
> >  include/linux/irqchip.h | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/irqchip.h b/include/linux/irqchip.h
> > index 950e4b2458f0..1f464fd10df0 100644
> > --- a/include/linux/irqchip.h
> > +++ b/include/linux/irqchip.h
> > @@ -39,8 +39,14 @@
> >   * @fn: initialization function
> >   */
> >  #define IRQCHIP_ACPI_DECLARE(name, subtable, validate, data, fn)	\
> > -	ACPI_DECLARE_PROBE_ENTRY(irqchip, name, ACPI_SIG_MADT, 		\
> > -				 subtable, validate, data, fn)
> > +	static const struct acpi_probe_entry __acpi_probe_##name	\
> > +		__used __section(__irqchip_acpi_probe_table) = {	\
> > +			.id = ACPI_SIG_MADT,				\
> > +			.type = subtable,				\
> > +			.subtable_valid = validate,			\
> > +			.probe_subtbl = (acpi_tbl_entry_handler)fn,	\
> > +			.driver_data = data,				\
> > +		}
> >
>
> I'd rather you add an ACPI_DECLARE_SUBTABLE_PROBE_ENTRY to acpi.h, and
> use that here so that we can keep the ACPI gunk in a single place.

Ok, I will do the changes you suggested and I will resend a new version.
Later, I will also send a series to clean up the checkpatch warnings and
errors for the acpi.h header.

> >  #ifdef CONFIG_IRQCHIP
> >  void irqchip_init(void);
> > --
> > 2.20.1
> >
> >
>
> Thanks,
>
> 	M.
> --
> Jazz is not dead. It just smells funny...

Thanks,
Oscar Carter
Marc Zyngier May 24, 2020, 4:16 p.m. UTC | #3
On 2020-05-24 17:06, Oscar Carter wrote:
> Hi Marc,
> 
> On Sun, May 24, 2020 at 12:46:34PM +0100, Marc Zyngier wrote:
>> On Sun, 24 May 2020 10:09:10 +0200
>> Oscar Carter <oscar.carter@gmx.com> wrote:
>> 
>> Hi Oscar,
>> 
>> Thanks for this. Comments below.
>> 
>> > In an effort to enable -Wcast-function-type in the top-level Makefile to
>> > support Control Flow Integrity builds, remove all the function callback
>> > casts.
>> >
>> > To do this, modify the IRQCHIP_ACPI_DECLARE macro initializing the
>> > acpi_probe_entry struct directly instead of use the existent macro
>> > ACPI_DECLARE_PROBE_ENTRY.
>> >
>> > In this new initialization use the probe_subtbl field instead of the
>> > probe_table field use in the ACPI_DECLARE_PROBE_ENTRY macro.
>> 
>> Please add *why* this is a valid transformation (probe_table and
>> probe_subtbl are part of a union).
> 
> Ok, I will add a more detailed explanation.
> 
>> > Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
>> > ---
>> >  include/linux/irqchip.h | 10 ++++++++--
>> >  1 file changed, 8 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/include/linux/irqchip.h b/include/linux/irqchip.h
>> > index 950e4b2458f0..1f464fd10df0 100644
>> > --- a/include/linux/irqchip.h
>> > +++ b/include/linux/irqchip.h
>> > @@ -39,8 +39,14 @@
>> >   * @fn: initialization function
>> >   */
>> >  #define IRQCHIP_ACPI_DECLARE(name, subtable, validate, data, fn)	\
>> > -	ACPI_DECLARE_PROBE_ENTRY(irqchip, name, ACPI_SIG_MADT, 		\
>> > -				 subtable, validate, data, fn)
>> > +	static const struct acpi_probe_entry __acpi_probe_##name	\
>> > +		__used __section(__irqchip_acpi_probe_table) = {	\
>> > +			.id = ACPI_SIG_MADT,				\
>> > +			.type = subtable,				\
>> > +			.subtable_valid = validate,			\
>> > +			.probe_subtbl = (acpi_tbl_entry_handler)fn,	\
>> > +			.driver_data = data,				\
>> > +		}
>> >
>> 
>> I'd rather you add an ACPI_DECLARE_SUBTABLE_PROBE_ENTRY to acpi.h, and
>> use that here so that we can keep the ACPI gunk in a single place.
> 
> Ok, I will do the changes you suggested and I will resend a new 
> version.
> Later, I will also send a series to clean up the checkpatch warnings 
> and
> errors for the acpi.h header.

Not necessarily a good idea. Churn for the sake of keeping checkpatch
at bay is pretty pointless. Do fix bugs if you spot any, but please
exercise judgement.

Thanks,

         M.
Oscar Carter May 26, 2020, 5:32 p.m. UTC | #4
Hi Marc,

On Sun, May 24, 2020 at 05:16:41PM +0100, Marc Zyngier wrote:
> On 2020-05-24 17:06, Oscar Carter wrote:
>
> > Ok, I will do the changes you suggested and I will resend a new version.
> > Later, I will also send a series to clean up the checkpatch warnings and
> > errors for the acpi.h header.
>
> Not necessarily a good idea. Churn for the sake of keeping checkpatch
> at bay is pretty pointless. Do fix bugs if you spot any, but please
> exercise judgement.

Ok, thanks for your advise. For now I will only fix the part related to this
patch.

> Thanks,
>
>         M.
> --
> Jazz is not dead. It just smells funny...

Thanks,
Oscar
diff mbox series

Patch

diff --git a/include/linux/irqchip.h b/include/linux/irqchip.h
index 950e4b2458f0..1f464fd10df0 100644
--- a/include/linux/irqchip.h
+++ b/include/linux/irqchip.h
@@ -39,8 +39,14 @@ 
  * @fn: initialization function
  */
 #define IRQCHIP_ACPI_DECLARE(name, subtable, validate, data, fn)	\
-	ACPI_DECLARE_PROBE_ENTRY(irqchip, name, ACPI_SIG_MADT, 		\
-				 subtable, validate, data, fn)
+	static const struct acpi_probe_entry __acpi_probe_##name	\
+		__used __section(__irqchip_acpi_probe_table) = {	\
+			.id = ACPI_SIG_MADT,				\
+			.type = subtable,				\
+			.subtable_valid = validate,			\
+			.probe_subtbl = (acpi_tbl_entry_handler)fn,	\
+			.driver_data = data,				\
+		}

 #ifdef CONFIG_IRQCHIP
 void irqchip_init(void);