[KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations

[KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations

[Andi Drebes]

This patch replaces lines in the acpi subsystem that use
sizeof/sizeof to determine the size of an array with the
ARRAY_SIZE(x) macro.

Signed-off-by: Andi Drebes <lists-receive@programmierforen.de>
---
Compile tested with allyesconfig on i386.
Diffed against Linus' git-tree.


diff --git a/drivers/acpi/resources/rsdump.c b/drivers/acpi/resources/rsdump.c
index 46da116..7b8e12d 100644
--- a/drivers/acpi/resources/rsdump.c
+++ b/drivers/acpi/resources/rsdump.c
@@ -76,7 +76,7 @@ acpi_rs_dump_descriptor(void *resource, struct acpi_rsdump_info *table);
 
 #define ACPI_RSD_OFFSET(f)          (u8) ACPI_OFFSET (union acpi_resource_data,f)
 #define ACPI_PRT_OFFSET(f)          (u8) ACPI_OFFSET (struct acpi_pci_routing_table,f)
-#define ACPI_RSD_TABLE_SIZE(name)   (sizeof(name) / sizeof (struct acpi_rsdump_info))
+#define ACPI_RSD_TABLE_SIZE(name)   (ARRAY_SIZE(name))
 
 /*******************************************************************************
  *
diff --git a/drivers/acpi/tables/tbfadt.c b/drivers/acpi/tables/tbfadt.c
index 1285e91..4d59de2 100644
--- a/drivers/acpi/tables/tbfadt.c
+++ b/drivers/acpi/tables/tbfadt.c
@@ -104,7 +104,7 @@ static struct acpi_fadt_info fadt_info_table[] = {
 	 ACPI_FADT_OFFSET(gpe1_block_length), ACPI_FADT_SEPARATE_LENGTH}
 };
 
-#define ACPI_FADT_INFO_ENTRIES        (sizeof (fadt_info_table) / sizeof (struct acpi_fadt_info))
+#define ACPI_FADT_INFO_ENTRIES        (ARRAY_SIZE(fadt_info_table))
 
 /*******************************************************************************
  *
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

[KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations replaced with ARRAY_SIZE

[Andi Drebes]

This patch replaces lines in the acpi subsystem that use
sizeof/sizeof to determine the size of an array with the
ARRAY_SIZE(x) macro.

Signed-off-by: Andi Drebes <lists-receive@programmierforen.de>
---
Compile tested with allyesconfig on i386.
Diffed against Linus' git-tree.


diff --git a/drivers/acpi/resources/rsdump.c b/drivers/acpi/resources/rsdump.c
index 46da116..7b8e12d 100644
--- a/drivers/acpi/resources/rsdump.c
+++ b/drivers/acpi/resources/rsdump.c
@@ -76,7 +76,7 @@ acpi_rs_dump_descriptor(void *resource, struct acpi_rsdump_info *table);
 
 #define ACPI_RSD_OFFSET(f)          (u8) ACPI_OFFSET (union acpi_resource_data,f)
 #define ACPI_PRT_OFFSET(f)          (u8) ACPI_OFFSET (struct acpi_pci_routing_table,f)
-#define ACPI_RSD_TABLE_SIZE(name)   (sizeof(name) / sizeof (struct acpi_rsdump_info))
+#define ACPI_RSD_TABLE_SIZE(name)   (ARRAY_SIZE(name))
 
 /*******************************************************************************
  *
diff --git a/drivers/acpi/tables/tbfadt.c b/drivers/acpi/tables/tbfadt.c
index 1285e91..4d59de2 100644
--- a/drivers/acpi/tables/tbfadt.c
+++ b/drivers/acpi/tables/tbfadt.c
@@ -104,7 +104,7 @@ static struct acpi_fadt_info fadt_info_table[] = {
 	 ACPI_FADT_OFFSET(gpe1_block_length), ACPI_FADT_SEPARATE_LENGTH}
 };
 
-#define ACPI_FADT_INFO_ENTRIES        (sizeof (fadt_info_table) / sizeof (struct acpi_fadt_info))
+#define ACPI_FADT_INFO_ENTRIES        (ARRAY_SIZE(fadt_info_table))
 
 /*******************************************************************************
  *

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations

[Robert P. J. Day]

On Sat, 26 May 2007, Andi Drebes wrote:

> This patch replaces lines in the acpi subsystem that use
> sizeof/sizeof to determine the size of an array with the
> ARRAY_SIZE(x) macro.
>
> Signed-off-by: Andi Drebes <lists-receive@programmierforen.de>
> ---
> Compile tested with allyesconfig on i386.
> Diffed against Linus' git-tree.
>
>
> diff --git a/drivers/acpi/resources/rsdump.c b/drivers/acpi/resources/rsdump.c
> index 46da116..7b8e12d 100644
> --- a/drivers/acpi/resources/rsdump.c
> +++ b/drivers/acpi/resources/rsdump.c
> @@ -76,7 +76,7 @@ acpi_rs_dump_descriptor(void *resource, struct acpi_rsdump_info *table);
>
>  #define ACPI_RSD_OFFSET(f)          (u8) ACPI_OFFSET (union acpi_resource_data,f)
>  #define ACPI_PRT_OFFSET(f)          (u8) ACPI_OFFSET (struct acpi_pci_routing_table,f)
> -#define ACPI_RSD_TABLE_SIZE(name)   (sizeof(name) / sizeof (struct acpi_rsdump_info))
> +#define ACPI_RSD_TABLE_SIZE(name)   (ARRAY_SIZE(name))
>
>  /*******************************************************************************
>   *

  normally, what i would do in a case like the above is delete the
macro ACPI_RSD_TABLE_SIZE entirely, and replace all invocations of it
with a direct invocation of ARRAY_SIZE.  there seems to be little
value in defining a whole new macro whose only purpose in life is to
do what an already-existing macro does just as well and with more
clarity.

  but there's something else about that file that's just a bit weird.
note the form of most of those array definitions:

struct acpi_rsdump_info acpi_rs_dump_irq[6] = {
        {ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_irq), "IRQ", NULL},
        {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(irq.triggering), "Triggering",
         acpi_gbl_he_decode},
	...

  what's the point of **hard-coding** the array size as "6", only to
use a macro in the very next line to calculate that same value?  and
that's done throughout those array definitions.  why not just leave
the hard-coded array size out, and let the macro take care of things?

rday
-- 
====================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
====================================
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations replaced with ARRAY_SIZE

[Robert P. J. Day]

On Sat, 26 May 2007, Andi Drebes wrote:

> This patch replaces lines in the acpi subsystem that use
> sizeof/sizeof to determine the size of an array with the
> ARRAY_SIZE(x) macro.
>
> Signed-off-by: Andi Drebes <lists-receive@programmierforen.de>
> ---
> Compile tested with allyesconfig on i386.
> Diffed against Linus' git-tree.
>
>
> diff --git a/drivers/acpi/resources/rsdump.c b/drivers/acpi/resources/rsdump.c
> index 46da116..7b8e12d 100644
> --- a/drivers/acpi/resources/rsdump.c
> +++ b/drivers/acpi/resources/rsdump.c
> @@ -76,7 +76,7 @@ acpi_rs_dump_descriptor(void *resource, struct acpi_rsdump_info *table);
>
>  #define ACPI_RSD_OFFSET(f)          (u8) ACPI_OFFSET (union acpi_resource_data,f)
>  #define ACPI_PRT_OFFSET(f)          (u8) ACPI_OFFSET (struct acpi_pci_routing_table,f)
> -#define ACPI_RSD_TABLE_SIZE(name)   (sizeof(name) / sizeof (struct acpi_rsdump_info))
> +#define ACPI_RSD_TABLE_SIZE(name)   (ARRAY_SIZE(name))
>
>  /*******************************************************************************
>   *

  normally, what i would do in a case like the above is delete the
macro ACPI_RSD_TABLE_SIZE entirely, and replace all invocations of it
with a direct invocation of ARRAY_SIZE.  there seems to be little
value in defining a whole new macro whose only purpose in life is to
do what an already-existing macro does just as well and with more
clarity.

  but there's something else about that file that's just a bit weird.
note the form of most of those array definitions:

struct acpi_rsdump_info acpi_rs_dump_irq[6] = {
        {ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_irq), "IRQ", NULL},
        {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(irq.triggering), "Triggering",
         acpi_gbl_he_decode},
	...

  what's the point of **hard-coding** the array size as "6", only to
use a macro in the very next line to calculate that same value?  and
that's done throughout those array definitions.  why not just leave
the hard-coded array size out, and let the macro take care of things?

rday
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations

[Andi Drebes]

<snip>
> > diff --git a/drivers/acpi/resources/rsdump.c b/drivers/acpi/resources/rsdump.c
> > index 46da116..7b8e12d 100644
> > --- a/drivers/acpi/resources/rsdump.c
> > +++ b/drivers/acpi/resources/rsdump.c
> > @@ -76,7 +76,7 @@ acpi_rs_dump_descriptor(void *resource, struct acpi_rsdump_info *table);
> >
> >  #define ACPI_RSD_OFFSET(f)          (u8) ACPI_OFFSET (union acpi_resource_data,f)
> >  #define ACPI_PRT_OFFSET(f)          (u8) ACPI_OFFSET (struct acpi_pci_routing_table,f)
> > -#define ACPI_RSD_TABLE_SIZE(name)   (sizeof(name) / sizeof (struct acpi_rsdump_info))
> > +#define ACPI_RSD_TABLE_SIZE(name)   (ARRAY_SIZE(name))
> >
> >  /*******************************************************************************
> >   *
> 
>   normally, what i would do in a case like the above is delete the
> macro ACPI_RSD_TABLE_SIZE entirely, and replace all invocations of it
> with a direct invocation of ARRAY_SIZE.  there seems to be little
> value in defining a whole new macro whose only purpose in life is to
> do what an already-existing macro does just as well and with more
> clarity.
OK. I just replaced all invocations of ACPI_RSD_TABLE_SIZE with ARRAY_SIZE
and removed the definition.

>   but there's something else about that file that's just a bit weird.
<snip>
>   what's the point of **hard-coding** the array size as "6", only to
> use a macro in the very next line to calculate that same value?  and
> that's done throughout those array definitions.  why not just leave
> the hard-coded array size out, and let the macro take care of things?
I tried to figure out why the value is hardcoded in the _first entry_. Actually, the acpi_rs_dump_*
array definitions are used in drivers/acpi/resources/rsinfo.c in another array
called acpi_gbl_dump_resource_dispatch (Line 124). They are also defined
as externel variables in include/acpi/acresrc.h. But I think the point of the
definitions in the header file is just to make them accessible fro rsinfo.c
(because the only references in drivers/acpi/ are in drivers/acpi/resources/rsinfo.c
and drivers/acpi/resources/rsdump.c (where they are defined)).
acpi_gbl_dump_resource_dispatch seems o be referenced only once. That is in
drivers/acpi/resources/rsdump.c in the function called acpi_rs_dump_resource_list:

acpi_rs_dump_descriptor(&resource_list->data,
					acpi_gbl_dump_resource_dispatch[type]);

acpi_rs_dump_descriptor is defined in the same file and finally uses the table.
The hardcoded sizes of the arrays are used to walk through the array. There's
also a comment about that:

/* First table entry must contain the table length (# of table entries) */

As acpi_rs_dump_descriptor only gets a pointer to an array like that, it's
impossible for it to determine the size (or am I wrong?). So finally, I think
the hardcoded values are really needed. What I dislike is that an ordinary
acpi_rsdump_info entry in the array is used to store the size. So
acpi_rsdump_info.offset is used in two ways. For the first entry this is the
number of entries in the array. For all the following entries it is a real offset.
Perhaps one should use a struct that contains an array for the following entries
instead of using an array directly (or is this a common practice?).
To get back to robert's comment: I think also that the hardcoded values
*in brackets* should be removed. I couldn't find any code that relies on that.
To determine the size, the first entry is used (see above).

However, there's another thing that confuses me. I tried to figure out how
drivers/acpi/resources/rsdump.c is actually compiled. I inspected the Makefile
which says:

obj-y := rsaddr.o rscreate.o rsinfo.o rsio.o rslist.o rsmisc.o rsxface.o \
	 rscalc.o  rsirq.o  rsmemory.o  rsutils.o

obj-$(ACPI_FUTURE_USAGE) += rsdump.o

I couldn't find ACPI_FUTURE_USAGE in the configuration. I tried to add
the object file diretly to obj-y which causes a lot of errors. I found this post
when I searched the web for more information about the symbol:

http://mail.nl.linux.org/kernelnewbies/2006-07/msg00250.html

The message is pretty old (July 2006), but it endorses my assumption that
this code isn't actually used. This is somewhat weird to me. Why is there
code in the kernel tree that is reserved for future use. And why does it live
there for already a year? I would really like to test my changes (even only by
compilation).

Here's a new patch that totally removes the ACPI_RSD_TABLE_SIZE macro,
aswell as the hardcoded array sizes in brackets at the beginning of a definition.
In contrast to the original patch, this one only affects rsdump.c (I split this, because
there's another issue about tbfadt.c pointed out by Richard Knutsson. The other
changes will be in a reply to his message)

	Cheers,
		Andi

The changes haven't been tested yet (see above)
Signed-off-by: Andi Drebes <lists-receive@programmierforen.de>
--


diff --git a/drivers/acpi/resources/rsdump.c b/drivers/acpi/resources/rsdump.c
index 46da116..67a7511 100644
--- a/drivers/acpi/resources/rsdump.c
+++ b/drivers/acpi/resources/rsdump.c
@@ -76,7 +76,6 @@ acpi_rs_dump_descriptor(void *resource, struct acpi_rsdump_info *table);
 
 #define ACPI_RSD_OFFSET(f)          (u8) ACPI_OFFSET (union acpi_resource_data,f)
 #define ACPI_PRT_OFFSET(f)          (u8) ACPI_OFFSET (struct acpi_pci_routing_table,f)
-#define ACPI_RSD_TABLE_SIZE(name)   (sizeof(name) / sizeof (struct acpi_rsdump_info))
 
 /*******************************************************************************
  *
@@ -87,8 +86,8 @@ acpi_rs_dump_descriptor(void *resource, struct acpi_rsdump_info *table);
  *
  ******************************************************************************/
 
-struct acpi_rsdump_info acpi_rs_dump_irq[6] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_irq), "IRQ", NULL},
+struct acpi_rsdump_info acpi_rs_dump_irq[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_irq), "IRQ", NULL},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(irq.triggering), "Triggering",
 	 acpi_gbl_he_decode},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(irq.polarity), "Polarity",
@@ -101,8 +100,8 @@ struct acpi_rsdump_info acpi_rs_dump_irq[6] = {
 	 "Interrupt List", NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_dma[6] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_dma), "DMA", NULL},
+struct acpi_rsdump_info acpi_rs_dump_dma[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_dma), "DMA", NULL},
 	{ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET(dma.type), "Speed",
 	 acpi_gbl_typ_decode},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(dma.bus_master), "Mastering",
@@ -115,8 +114,8 @@ struct acpi_rsdump_info acpi_rs_dump_dma[6] = {
 	 NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_start_dpf[3] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_start_dpf),
+struct acpi_rsdump_info acpi_rs_dump_start_dpf[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_start_dpf),
 	 "Start-Dependent-Functions", NULL},
 	{ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET(start_dpf.compatibility_priority),
 	 "Compatibility Priority", acpi_gbl_config_decode},
@@ -124,13 +123,13 @@ struct acpi_rsdump_info acpi_rs_dump_start_dpf[3] = {
 	 "Performance/Robustness", acpi_gbl_config_decode}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_end_dpf[1] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_end_dpf),
+struct acpi_rsdump_info acpi_rs_dump_end_dpf[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_end_dpf),
 	 "End-Dependent-Functions", NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_io[6] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_io), "I/O", NULL},
+struct acpi_rsdump_info acpi_rs_dump_io[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_io), "I/O", NULL},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(io.io_decode), "Address Decoding",
 	 acpi_gbl_io_decode},
 	{ACPI_RSD_UINT16, ACPI_RSD_OFFSET(io.minimum), "Address Minimum", NULL},
@@ -140,29 +139,29 @@ struct acpi_rsdump_info acpi_rs_dump_io[6] = {
 	 NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_fixed_io[3] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_fixed_io),
+struct acpi_rsdump_info acpi_rs_dump_fixed_io[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_fixed_io),
 	 "Fixed I/O", NULL},
 	{ACPI_RSD_UINT16, ACPI_RSD_OFFSET(fixed_io.address), "Address", NULL},
 	{ACPI_RSD_UINT8, ACPI_RSD_OFFSET(fixed_io.address_length),
 	 "Address Length", NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_vendor[3] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_vendor),
+struct acpi_rsdump_info acpi_rs_dump_vendor[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_vendor),
 	 "Vendor Specific", NULL},
 	{ACPI_RSD_UINT16, ACPI_RSD_OFFSET(vendor.byte_length), "Length", NULL},
 	{ACPI_RSD_LONGLIST, ACPI_RSD_OFFSET(vendor.byte_data[0]), "Vendor Data",
 	 NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_end_tag[1] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_end_tag), "EndTag",
+struct acpi_rsdump_info acpi_rs_dump_end_tag[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_end_tag), "EndTag",
 	 NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_memory24[6] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_memory24),
+struct acpi_rsdump_info acpi_rs_dump_memory24[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_memory24),
 	 "24-Bit Memory Range", NULL},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(memory24.write_protect),
 	 "Write Protect", acpi_gbl_rw_decode},
@@ -176,8 +175,8 @@ struct acpi_rsdump_info acpi_rs_dump_memory24[6] = {
 	 "Address Length", NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_memory32[6] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_memory32),
+struct acpi_rsdump_info acpi_rs_dump_memory32[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_memory32),
 	 "32-Bit Memory Range", NULL},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(memory32.write_protect),
 	 "Write Protect", acpi_gbl_rw_decode},
@@ -191,8 +190,8 @@ struct acpi_rsdump_info acpi_rs_dump_memory32[6] = {
 	 "Address Length", NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_fixed_memory32[4] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_fixed_memory32),
+struct acpi_rsdump_info acpi_rs_dump_fixed_memory32[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_fixed_memory32),
 	 "32-Bit Fixed Memory Range", NULL},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(fixed_memory32.write_protect),
 	 "Write Protect", acpi_gbl_rw_decode},
@@ -202,8 +201,8 @@ struct acpi_rsdump_info acpi_rs_dump_fixed_memory32[4] = {
 	 "Address Length", NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_address16[8] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_address16),
+struct acpi_rsdump_info acpi_rs_dump_address16[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_address16),
 	 "16-Bit WORD Address Space", NULL},
 	{ACPI_RSD_ADDRESS, 0, NULL, NULL},
 	{ACPI_RSD_UINT16, ACPI_RSD_OFFSET(address16.granularity), "Granularity",
@@ -219,8 +218,8 @@ struct acpi_rsdump_info acpi_rs_dump_address16[8] = {
 	{ACPI_RSD_SOURCE, ACPI_RSD_OFFSET(address16.resource_source), NULL, NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_address32[8] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_address32),
+struct acpi_rsdump_info acpi_rs_dump_address32[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_address32),
 	 "32-Bit DWORD Address Space", NULL},
 	{ACPI_RSD_ADDRESS, 0, NULL, NULL},
 	{ACPI_RSD_UINT32, ACPI_RSD_OFFSET(address32.granularity), "Granularity",
@@ -236,8 +235,8 @@ struct acpi_rsdump_info acpi_rs_dump_address32[8] = {
 	{ACPI_RSD_SOURCE, ACPI_RSD_OFFSET(address32.resource_source), NULL, NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_address64[8] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_address64),
+struct acpi_rsdump_info acpi_rs_dump_address64[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_address64),
 	 "64-Bit QWORD Address Space", NULL},
 	{ACPI_RSD_ADDRESS, 0, NULL, NULL},
 	{ACPI_RSD_UINT64, ACPI_RSD_OFFSET(address64.granularity), "Granularity",
@@ -253,8 +252,8 @@ struct acpi_rsdump_info acpi_rs_dump_address64[8] = {
 	{ACPI_RSD_SOURCE, ACPI_RSD_OFFSET(address64.resource_source), NULL, NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_ext_address64[8] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_ext_address64),
+struct acpi_rsdump_info acpi_rs_dump_ext_address64[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_ext_address64),
 	 "64-Bit Extended Address Space", NULL},
 	{ACPI_RSD_ADDRESS, 0, NULL, NULL},
 	{ACPI_RSD_UINT64, ACPI_RSD_OFFSET(ext_address64.granularity),
@@ -271,8 +270,8 @@ struct acpi_rsdump_info acpi_rs_dump_ext_address64[8] = {
 	 "Type-Specific Attribute", NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_ext_irq[8] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_ext_irq),
+struct acpi_rsdump_info acpi_rs_dump_ext_irq[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_ext_irq),
 	 "Extended IRQ", NULL},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(extended_irq.producer_consumer),
 	 "Type", acpi_gbl_consume_decode},
@@ -290,8 +289,8 @@ struct acpi_rsdump_info acpi_rs_dump_ext_irq[8] = {
 	 "Interrupt List", NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_generic_reg[6] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_generic_reg),
+struct acpi_rsdump_info acpi_rs_dump_generic_reg[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_generic_reg),
 	 "Generic Register", NULL},
 	{ACPI_RSD_UINT8, ACPI_RSD_OFFSET(generic_reg.space_id), "Space ID",
 	 NULL},
@@ -307,8 +306,8 @@ struct acpi_rsdump_info acpi_rs_dump_generic_reg[6] = {
 /*
  * Tables used for common address descriptor flag fields
  */
-static struct acpi_rsdump_info acpi_rs_dump_general_flags[5] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_general_flags), NULL,
+static struct acpi_rsdump_info acpi_rs_dump_general_flags[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_general_flags), NULL,
 	 NULL},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(address.producer_consumer),
 	 "Consumer/Producer", acpi_gbl_consume_decode},
@@ -320,8 +319,8 @@ static struct acpi_rsdump_info acpi_rs_dump_general_flags[5] = {
 	 "Max Relocatability", acpi_gbl_max_decode}
 };
 
-static struct acpi_rsdump_info acpi_rs_dump_memory_flags[5] = {
-	{ACPI_RSD_LITERAL, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_memory_flags),
+static struct acpi_rsdump_info acpi_rs_dump_memory_flags[] = {
+	{ACPI_RSD_LITERAL, ARRAY_SIZE(acpi_rs_dump_memory_flags),
 	 "Resource Type", (void *)"Memory Range"},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(address.info.mem.write_protect),
 	 "Write Protect", acpi_gbl_rw_decode},
@@ -333,8 +332,8 @@ static struct acpi_rsdump_info acpi_rs_dump_memory_flags[5] = {
 	 "Translation", acpi_gbl_ttp_decode}
 };
 
-static struct acpi_rsdump_info acpi_rs_dump_io_flags[4] = {
-	{ACPI_RSD_LITERAL, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_io_flags),
+static struct acpi_rsdump_info acpi_rs_dump_io_flags[] = {
+	{ACPI_RSD_LITERAL, ARRAY_SIZE(acpi_rs_dump_io_flags),
 	 "Resource Type", (void *)"I/O Range"},
 	{ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET(address.info.io.range_type),
 	 "Range Type", acpi_gbl_rng_decode},
@@ -347,8 +346,8 @@ static struct acpi_rsdump_info acpi_rs_dump_io_flags[4] = {
 /*
  * Table used to dump _PRT contents
  */
-static struct acpi_rsdump_info acpi_rs_dump_prt[5] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_prt), NULL, NULL},
+static struct acpi_rsdump_info acpi_rs_dump_prt[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_prt), NULL, NULL},
 	{ACPI_RSD_UINT64, ACPI_PRT_OFFSET(address), "Address", NULL},
 	{ACPI_RSD_UINT32, ACPI_PRT_OFFSET(pin), "Pin", NULL},
 	{ACPI_RSD_STRING, ACPI_PRT_OFFSET(source[0]), "Source", NULL},
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations replaced with ARRAY_SIZE

[Andi Drebes]

<snip>
> > diff --git a/drivers/acpi/resources/rsdump.c b/drivers/acpi/resources/rsdump.c
> > index 46da116..7b8e12d 100644
> > --- a/drivers/acpi/resources/rsdump.c
> > +++ b/drivers/acpi/resources/rsdump.c
> > @@ -76,7 +76,7 @@ acpi_rs_dump_descriptor(void *resource, struct acpi_rsdump_info *table);
> >
> >  #define ACPI_RSD_OFFSET(f)          (u8) ACPI_OFFSET (union acpi_resource_data,f)
> >  #define ACPI_PRT_OFFSET(f)          (u8) ACPI_OFFSET (struct acpi_pci_routing_table,f)
> > -#define ACPI_RSD_TABLE_SIZE(name)   (sizeof(name) / sizeof (struct acpi_rsdump_info))
> > +#define ACPI_RSD_TABLE_SIZE(name)   (ARRAY_SIZE(name))
> >
> >  /*******************************************************************************
> >   *
> 
>   normally, what i would do in a case like the above is delete the
> macro ACPI_RSD_TABLE_SIZE entirely, and replace all invocations of it
> with a direct invocation of ARRAY_SIZE.  there seems to be little
> value in defining a whole new macro whose only purpose in life is to
> do what an already-existing macro does just as well and with more
> clarity.
OK. I just replaced all invocations of ACPI_RSD_TABLE_SIZE with ARRAY_SIZE
and removed the definition.

>   but there's something else about that file that's just a bit weird.
<snip>
>   what's the point of **hard-coding** the array size as "6", only to
> use a macro in the very next line to calculate that same value?  and
> that's done throughout those array definitions.  why not just leave
> the hard-coded array size out, and let the macro take care of things?
I tried to figure out why the value is hardcoded in the _first entry_. Actually, the acpi_rs_dump_*
array definitions are used in drivers/acpi/resources/rsinfo.c in another array
called acpi_gbl_dump_resource_dispatch (Line 124). They are also defined
as externel variables in include/acpi/acresrc.h. But I think the point of the
definitions in the header file is just to make them accessible fro rsinfo.c
(because the only references in drivers/acpi/ are in drivers/acpi/resources/rsinfo.c
and drivers/acpi/resources/rsdump.c (where they are defined)).
acpi_gbl_dump_resource_dispatch seems o be referenced only once. That is in
drivers/acpi/resources/rsdump.c in the function called acpi_rs_dump_resource_list:

acpi_rs_dump_descriptor(&resource_list->data,
					acpi_gbl_dump_resource_dispatch[type]);

acpi_rs_dump_descriptor is defined in the same file and finally uses the table.
The hardcoded sizes of the arrays are used to walk through the array. There's
also a comment about that:

/* First table entry must contain the table length (# of table entries) */

As acpi_rs_dump_descriptor only gets a pointer to an array like that, it's
impossible for it to determine the size (or am I wrong?). So finally, I think
the hardcoded values are really needed. What I dislike is that an ordinary
acpi_rsdump_info entry in the array is used to store the size. So
acpi_rsdump_info.offset is used in two ways. For the first entry this is the
number of entries in the array. For all the following entries it is a real offset.
Perhaps one should use a struct that contains an array for the following entries
instead of using an array directly (or is this a common practice?).
To get back to robert's comment: I think also that the hardcoded values
*in brackets* should be removed. I couldn't find any code that relies on that.
To determine the size, the first entry is used (see above).

However, there's another thing that confuses me. I tried to figure out how
drivers/acpi/resources/rsdump.c is actually compiled. I inspected the Makefile
which says:

obj-y := rsaddr.o rscreate.o rsinfo.o rsio.o rslist.o rsmisc.o rsxface.o \
	 rscalc.o  rsirq.o  rsmemory.o  rsutils.o

obj-$(ACPI_FUTURE_USAGE) += rsdump.o

I couldn't find ACPI_FUTURE_USAGE in the configuration. I tried to add
the object file diretly to obj-y which causes a lot of errors. I found this post
when I searched the web for more information about the symbol:

http://mail.nl.linux.org/kernelnewbies/2006-07/msg00250.html

The message is pretty old (July 2006), but it endorses my assumption that
this code isn't actually used. This is somewhat weird to me. Why is there
code in the kernel tree that is reserved for future use. And why does it live
there for already a year? I would really like to test my changes (even only by
compilation).

Here's a new patch that totally removes the ACPI_RSD_TABLE_SIZE macro,
aswell as the hardcoded array sizes in brackets at the beginning of a definition.
In contrast to the original patch, this one only affects rsdump.c (I split this, because
there's another issue about tbfadt.c pointed out by Richard Knutsson. The other
changes will be in a reply to his message)

	Cheers,
		Andi

The changes haven't been tested yet (see above)
Signed-off-by: Andi Drebes <lists-receive@programmierforen.de>
--


diff --git a/drivers/acpi/resources/rsdump.c b/drivers/acpi/resources/rsdump.c
index 46da116..67a7511 100644
--- a/drivers/acpi/resources/rsdump.c
+++ b/drivers/acpi/resources/rsdump.c
@@ -76,7 +76,6 @@ acpi_rs_dump_descriptor(void *resource, struct acpi_rsdump_info *table);
 
 #define ACPI_RSD_OFFSET(f)          (u8) ACPI_OFFSET (union acpi_resource_data,f)
 #define ACPI_PRT_OFFSET(f)          (u8) ACPI_OFFSET (struct acpi_pci_routing_table,f)
-#define ACPI_RSD_TABLE_SIZE(name)   (sizeof(name) / sizeof (struct acpi_rsdump_info))
 
 /*******************************************************************************
  *
@@ -87,8 +86,8 @@ acpi_rs_dump_descriptor(void *resource, struct acpi_rsdump_info *table);
  *
  ******************************************************************************/
 
-struct acpi_rsdump_info acpi_rs_dump_irq[6] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_irq), "IRQ", NULL},
+struct acpi_rsdump_info acpi_rs_dump_irq[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_irq), "IRQ", NULL},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(irq.triggering), "Triggering",
 	 acpi_gbl_he_decode},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(irq.polarity), "Polarity",
@@ -101,8 +100,8 @@ struct acpi_rsdump_info acpi_rs_dump_irq[6] = {
 	 "Interrupt List", NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_dma[6] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_dma), "DMA", NULL},
+struct acpi_rsdump_info acpi_rs_dump_dma[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_dma), "DMA", NULL},
 	{ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET(dma.type), "Speed",
 	 acpi_gbl_typ_decode},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(dma.bus_master), "Mastering",
@@ -115,8 +114,8 @@ struct acpi_rsdump_info acpi_rs_dump_dma[6] = {
 	 NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_start_dpf[3] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_start_dpf),
+struct acpi_rsdump_info acpi_rs_dump_start_dpf[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_start_dpf),
 	 "Start-Dependent-Functions", NULL},
 	{ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET(start_dpf.compatibility_priority),
 	 "Compatibility Priority", acpi_gbl_config_decode},
@@ -124,13 +123,13 @@ struct acpi_rsdump_info acpi_rs_dump_start_dpf[3] = {
 	 "Performance/Robustness", acpi_gbl_config_decode}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_end_dpf[1] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_end_dpf),
+struct acpi_rsdump_info acpi_rs_dump_end_dpf[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_end_dpf),
 	 "End-Dependent-Functions", NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_io[6] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_io), "I/O", NULL},
+struct acpi_rsdump_info acpi_rs_dump_io[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_io), "I/O", NULL},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(io.io_decode), "Address Decoding",
 	 acpi_gbl_io_decode},
 	{ACPI_RSD_UINT16, ACPI_RSD_OFFSET(io.minimum), "Address Minimum", NULL},
@@ -140,29 +139,29 @@ struct acpi_rsdump_info acpi_rs_dump_io[6] = {
 	 NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_fixed_io[3] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_fixed_io),
+struct acpi_rsdump_info acpi_rs_dump_fixed_io[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_fixed_io),
 	 "Fixed I/O", NULL},
 	{ACPI_RSD_UINT16, ACPI_RSD_OFFSET(fixed_io.address), "Address", NULL},
 	{ACPI_RSD_UINT8, ACPI_RSD_OFFSET(fixed_io.address_length),
 	 "Address Length", NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_vendor[3] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_vendor),
+struct acpi_rsdump_info acpi_rs_dump_vendor[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_vendor),
 	 "Vendor Specific", NULL},
 	{ACPI_RSD_UINT16, ACPI_RSD_OFFSET(vendor.byte_length), "Length", NULL},
 	{ACPI_RSD_LONGLIST, ACPI_RSD_OFFSET(vendor.byte_data[0]), "Vendor Data",
 	 NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_end_tag[1] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_end_tag), "EndTag",
+struct acpi_rsdump_info acpi_rs_dump_end_tag[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_end_tag), "EndTag",
 	 NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_memory24[6] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_memory24),
+struct acpi_rsdump_info acpi_rs_dump_memory24[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_memory24),
 	 "24-Bit Memory Range", NULL},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(memory24.write_protect),
 	 "Write Protect", acpi_gbl_rw_decode},
@@ -176,8 +175,8 @@ struct acpi_rsdump_info acpi_rs_dump_memory24[6] = {
 	 "Address Length", NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_memory32[6] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_memory32),
+struct acpi_rsdump_info acpi_rs_dump_memory32[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_memory32),
 	 "32-Bit Memory Range", NULL},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(memory32.write_protect),
 	 "Write Protect", acpi_gbl_rw_decode},
@@ -191,8 +190,8 @@ struct acpi_rsdump_info acpi_rs_dump_memory32[6] = {
 	 "Address Length", NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_fixed_memory32[4] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_fixed_memory32),
+struct acpi_rsdump_info acpi_rs_dump_fixed_memory32[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_fixed_memory32),
 	 "32-Bit Fixed Memory Range", NULL},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(fixed_memory32.write_protect),
 	 "Write Protect", acpi_gbl_rw_decode},
@@ -202,8 +201,8 @@ struct acpi_rsdump_info acpi_rs_dump_fixed_memory32[4] = {
 	 "Address Length", NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_address16[8] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_address16),
+struct acpi_rsdump_info acpi_rs_dump_address16[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_address16),
 	 "16-Bit WORD Address Space", NULL},
 	{ACPI_RSD_ADDRESS, 0, NULL, NULL},
 	{ACPI_RSD_UINT16, ACPI_RSD_OFFSET(address16.granularity), "Granularity",
@@ -219,8 +218,8 @@ struct acpi_rsdump_info acpi_rs_dump_address16[8] = {
 	{ACPI_RSD_SOURCE, ACPI_RSD_OFFSET(address16.resource_source), NULL, NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_address32[8] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_address32),
+struct acpi_rsdump_info acpi_rs_dump_address32[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_address32),
 	 "32-Bit DWORD Address Space", NULL},
 	{ACPI_RSD_ADDRESS, 0, NULL, NULL},
 	{ACPI_RSD_UINT32, ACPI_RSD_OFFSET(address32.granularity), "Granularity",
@@ -236,8 +235,8 @@ struct acpi_rsdump_info acpi_rs_dump_address32[8] = {
 	{ACPI_RSD_SOURCE, ACPI_RSD_OFFSET(address32.resource_source), NULL, NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_address64[8] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_address64),
+struct acpi_rsdump_info acpi_rs_dump_address64[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_address64),
 	 "64-Bit QWORD Address Space", NULL},
 	{ACPI_RSD_ADDRESS, 0, NULL, NULL},
 	{ACPI_RSD_UINT64, ACPI_RSD_OFFSET(address64.granularity), "Granularity",
@@ -253,8 +252,8 @@ struct acpi_rsdump_info acpi_rs_dump_address64[8] = {
 	{ACPI_RSD_SOURCE, ACPI_RSD_OFFSET(address64.resource_source), NULL, NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_ext_address64[8] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_ext_address64),
+struct acpi_rsdump_info acpi_rs_dump_ext_address64[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_ext_address64),
 	 "64-Bit Extended Address Space", NULL},
 	{ACPI_RSD_ADDRESS, 0, NULL, NULL},
 	{ACPI_RSD_UINT64, ACPI_RSD_OFFSET(ext_address64.granularity),
@@ -271,8 +270,8 @@ struct acpi_rsdump_info acpi_rs_dump_ext_address64[8] = {
 	 "Type-Specific Attribute", NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_ext_irq[8] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_ext_irq),
+struct acpi_rsdump_info acpi_rs_dump_ext_irq[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_ext_irq),
 	 "Extended IRQ", NULL},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(extended_irq.producer_consumer),
 	 "Type", acpi_gbl_consume_decode},
@@ -290,8 +289,8 @@ struct acpi_rsdump_info acpi_rs_dump_ext_irq[8] = {
 	 "Interrupt List", NULL}
 };
 
-struct acpi_rsdump_info acpi_rs_dump_generic_reg[6] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_generic_reg),
+struct acpi_rsdump_info acpi_rs_dump_generic_reg[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_generic_reg),
 	 "Generic Register", NULL},
 	{ACPI_RSD_UINT8, ACPI_RSD_OFFSET(generic_reg.space_id), "Space ID",
 	 NULL},
@@ -307,8 +306,8 @@ struct acpi_rsdump_info acpi_rs_dump_generic_reg[6] = {
 /*
  * Tables used for common address descriptor flag fields
  */
-static struct acpi_rsdump_info acpi_rs_dump_general_flags[5] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_general_flags), NULL,
+static struct acpi_rsdump_info acpi_rs_dump_general_flags[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_general_flags), NULL,
 	 NULL},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(address.producer_consumer),
 	 "Consumer/Producer", acpi_gbl_consume_decode},
@@ -320,8 +319,8 @@ static struct acpi_rsdump_info acpi_rs_dump_general_flags[5] = {
 	 "Max Relocatability", acpi_gbl_max_decode}
 };
 
-static struct acpi_rsdump_info acpi_rs_dump_memory_flags[5] = {
-	{ACPI_RSD_LITERAL, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_memory_flags),
+static struct acpi_rsdump_info acpi_rs_dump_memory_flags[] = {
+	{ACPI_RSD_LITERAL, ARRAY_SIZE(acpi_rs_dump_memory_flags),
 	 "Resource Type", (void *)"Memory Range"},
 	{ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET(address.info.mem.write_protect),
 	 "Write Protect", acpi_gbl_rw_decode},
@@ -333,8 +332,8 @@ static struct acpi_rsdump_info acpi_rs_dump_memory_flags[5] = {
 	 "Translation", acpi_gbl_ttp_decode}
 };
 
-static struct acpi_rsdump_info acpi_rs_dump_io_flags[4] = {
-	{ACPI_RSD_LITERAL, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_io_flags),
+static struct acpi_rsdump_info acpi_rs_dump_io_flags[] = {
+	{ACPI_RSD_LITERAL, ARRAY_SIZE(acpi_rs_dump_io_flags),
 	 "Resource Type", (void *)"I/O Range"},
 	{ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET(address.info.io.range_type),
 	 "Range Type", acpi_gbl_rng_decode},
@@ -347,8 +346,8 @@ static struct acpi_rsdump_info acpi_rs_dump_io_flags[4] = {
 /*
  * Table used to dump _PRT contents
  */
-static struct acpi_rsdump_info acpi_rs_dump_prt[5] = {
-	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_prt), NULL, NULL},
+static struct acpi_rsdump_info acpi_rs_dump_prt[] = {
+	{ACPI_RSD_TITLE, ARRAY_SIZE(acpi_rs_dump_prt), NULL, NULL},
 	{ACPI_RSD_UINT64, ACPI_PRT_OFFSET(address), "Address", NULL},
 	{ACPI_RSD_UINT32, ACPI_PRT_OFFSET(pin), "Pin", NULL},
 	{ACPI_RSD_STRING, ACPI_PRT_OFFSET(source[0]), "Source", NULL},

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations

[Richard Knutsson]

Andi Drebes wrote:
> This patch replaces lines in the acpi subsystem that use
> sizeof/sizeof to determine the size of an array with the
> ARRAY_SIZE(x) macro.
>
> Signed-off-by: Andi Drebes <lists-receive@programmierforen.de>
> ---
> Compile tested with allyesconfig on i386.
> Diffed against Linus' git-tree.
>
>
> diff --git a/drivers/acpi/resources/rsdump.c b/drivers/acpi/resources/rsdump.c
> index 46da116..7b8e12d 100644
> --- a/drivers/acpi/resources/rsdump.c
> +++ b/drivers/acpi/resources/rsdump.c
> @@ -76,7 +76,7 @@ acpi_rs_dump_descriptor(void *resource, struct acpi_rsdump_info *table);
>  
>  #define ACPI_RSD_OFFSET(f)          (u8) ACPI_OFFSET (union acpi_resource_data,f)
>  #define ACPI_PRT_OFFSET(f)          (u8) ACPI_OFFSET (struct acpi_pci_routing_table,f)
> -#define ACPI_RSD_TABLE_SIZE(name)   (sizeof(name) / sizeof (struct acpi_rsdump_info))
> +#define ACPI_RSD_TABLE_SIZE(name)   (ARRAY_SIZE(name))
>   
Any reason to not just replace ACPI_RSD_TABLE_SIZE with ARRAY_SIZE? Got 
just 21 instances of it in the file (+ the define) and no more in the 
rest of the tree.
>  
>  /*******************************************************************************
>   *
> diff --git a/drivers/acpi/tables/tbfadt.c b/drivers/acpi/tables/tbfadt.c
> index 1285e91..4d59de2 100644
> --- a/drivers/acpi/tables/tbfadt.c
> +++ b/drivers/acpi/tables/tbfadt.c
> @@ -104,7 +104,7 @@ static struct acpi_fadt_info fadt_info_table[] = {
>  	 ACPI_FADT_OFFSET(gpe1_block_length), ACPI_FADT_SEPARATE_LENGTH}
>  };
>  
> -#define ACPI_FADT_INFO_ENTRIES        (sizeof (fadt_info_table) / sizeof (struct acpi_fadt_info))
> +#define ACPI_FADT_INFO_ENTRIES        (ARRAY_SIZE(fadt_info_table))
>   
Normally I think this is ok when it is just a "constant", since the name 
of it may be more descriptive, but it is just used twice and I find 
(imho) ARRAY_SIZE(fadt_info_table) easier to understand.
>  
>  /*******************************************************************************
>   *
>   
cu
Richard Knutsson

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations replaced with ARRAY_SIZE

[Richard Knutsson]

Andi Drebes wrote:
> This patch replaces lines in the acpi subsystem that use
> sizeof/sizeof to determine the size of an array with the
> ARRAY_SIZE(x) macro.
>
> Signed-off-by: Andi Drebes <lists-receive@programmierforen.de>
> ---
> Compile tested with allyesconfig on i386.
> Diffed against Linus' git-tree.
>
>
> diff --git a/drivers/acpi/resources/rsdump.c b/drivers/acpi/resources/rsdump.c
> index 46da116..7b8e12d 100644
> --- a/drivers/acpi/resources/rsdump.c
> +++ b/drivers/acpi/resources/rsdump.c
> @@ -76,7 +76,7 @@ acpi_rs_dump_descriptor(void *resource, struct acpi_rsdump_info *table);
>  
>  #define ACPI_RSD_OFFSET(f)          (u8) ACPI_OFFSET (union acpi_resource_data,f)
>  #define ACPI_PRT_OFFSET(f)          (u8) ACPI_OFFSET (struct acpi_pci_routing_table,f)
> -#define ACPI_RSD_TABLE_SIZE(name)   (sizeof(name) / sizeof (struct acpi_rsdump_info))
> +#define ACPI_RSD_TABLE_SIZE(name)   (ARRAY_SIZE(name))
>   
Any reason to not just replace ACPI_RSD_TABLE_SIZE with ARRAY_SIZE? Got 
just 21 instances of it in the file (+ the define) and no more in the 
rest of the tree.
>  
>  /*******************************************************************************
>   *
> diff --git a/drivers/acpi/tables/tbfadt.c b/drivers/acpi/tables/tbfadt.c
> index 1285e91..4d59de2 100644
> --- a/drivers/acpi/tables/tbfadt.c
> +++ b/drivers/acpi/tables/tbfadt.c
> @@ -104,7 +104,7 @@ static struct acpi_fadt_info fadt_info_table[] = {
>  	 ACPI_FADT_OFFSET(gpe1_block_length), ACPI_FADT_SEPARATE_LENGTH}
>  };
>  
> -#define ACPI_FADT_INFO_ENTRIES        (sizeof (fadt_info_table) / sizeof (struct acpi_fadt_info))
> +#define ACPI_FADT_INFO_ENTRIES        (ARRAY_SIZE(fadt_info_table))
>   
Normally I think this is ok when it is just a "constant", since the name 
of it may be more descriptive, but it is just used twice and I find 
(imho) ARRAY_SIZE(fadt_info_table) easier to understand.
>  
>  /*******************************************************************************
>   *
>   
cu
Richard Knutsson

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations

[Andi Drebes]

<snip>
> > diff --git a/drivers/acpi/resources/rsdump.c b/drivers/acpi/resources/rsdump.c
> > index 46da116..7b8e12d 100644
> > --- a/drivers/acpi/resources/rsdump.c
> > +++ b/drivers/acpi/resources/rsdump.c
> > @@ -76,7 +76,7 @@ acpi_rs_dump_descriptor(void *resource, struct acpi_rsdump_info *table);
> >  
> >  #define ACPI_RSD_OFFSET(f)          (u8) ACPI_OFFSET (union acpi_resource_data,f)
> >  #define ACPI_PRT_OFFSET(f)          (u8) ACPI_OFFSET (struct acpi_pci_routing_table,f)
> > -#define ACPI_RSD_TABLE_SIZE(name)   (sizeof(name) / sizeof (struct acpi_rsdump_info))
> > +#define ACPI_RSD_TABLE_SIZE(name)   (ARRAY_SIZE(name))
> >   
> Any reason to not just replace ACPI_RSD_TABLE_SIZE with ARRAY_SIZE? Got 
> just 21 instances of it in the file (+ the define) and no more in the 
> rest of the tree.
I agree with that. Robert P. J. Day pointed out something similiar. For
a more verbose answer to that see the reply to his message.

> >  /*******************************************************************************
> >   *
> > diff --git a/drivers/acpi/tables/tbfadt.c b/drivers/acpi/tables/tbfadt.c
> > index 1285e91..4d59de2 100644
> > --- a/drivers/acpi/tables/tbfadt.c
> > +++ b/drivers/acpi/tables/tbfadt.c
> > @@ -104,7 +104,7 @@ static struct acpi_fadt_info fadt_info_table[] = {
> >  	 ACPI_FADT_OFFSET(gpe1_block_length), ACPI_FADT_SEPARATE_LENGTH}
> >  };
> >  
> > -#define ACPI_FADT_INFO_ENTRIES        (sizeof (fadt_info_table) / sizeof (struct acpi_fadt_info))
> > +#define ACPI_FADT_INFO_ENTRIES        (ARRAY_SIZE(fadt_info_table))
> >   
> Normally I think this is ok when it is just a "constant", since the name 
> of it may be more descriptive, but it is just used twice and I find 
> (imho) ARRAY_SIZE(fadt_info_table) easier to understand.
I totally agree with that. Here's a new patch that totally removes the ACPI_FADT_INFO_ENTRIES
macro. In contrast to the original patch, this one only affects tbfadt.c (I split this, because
there's another issue about rsdump.c pointed out by Robert P. J. Day. The other
changes will be in a reply to his message)

Tested by compilation.
Signed-off-by: Andi Drebes <lists-receive@programmierforen.de>
--

diff --git a/drivers/acpi/tables/tbfadt.c b/drivers/acpi/tables/tbfadt.c
index 1285e91..ba6d4a1 100644
--- a/drivers/acpi/tables/tbfadt.c
+++ b/drivers/acpi/tables/tbfadt.c
@@ -104,8 +104,6 @@ static struct acpi_fadt_info fadt_info_table[] = {
 	 ACPI_FADT_OFFSET(gpe1_block_length), ACPI_FADT_SEPARATE_LENGTH}
 };
 
-#define ACPI_FADT_INFO_ENTRIES        (sizeof (fadt_info_table) / sizeof (struct acpi_fadt_info))
-
 /*******************************************************************************
  *
  * FUNCTION:    acpi_tb_init_generic_address
@@ -295,7 +293,7 @@ static void acpi_tb_convert_fadt(void)
 	 * Expand the 32-bit V1.0 addresses to the 64-bit "X" generic address
 	 * structures as necessary.
 	 */
-	for (i = 0; i < ACPI_FADT_INFO_ENTRIES; i++) {
+	for (i = 0; i < ARRAY_SIZE(fadt_info_table); i++) {
 		target  		    ACPI_ADD_PTR(struct acpi_generic_address, &acpi_gbl_FADT,
 				 fadt_info_table[i].target);
@@ -392,7 +390,7 @@ static void acpi_tb_validate_fadt(void)
 
 	/* Examine all of the 64-bit extended address fields (X fields) */
 
-	for (i = 0; i < ACPI_FADT_INFO_ENTRIES; i++) {
+	for (i = 0; i < ARRAY_SIZE(fadt_info_table); i++) {
 
 		/* Generate pointers to the 32-bit and 64-bit addresses and get the length */
 
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations replaced with ARRAY_SIZE

[Andi Drebes]

<snip>
> > diff --git a/drivers/acpi/resources/rsdump.c b/drivers/acpi/resources/rsdump.c
> > index 46da116..7b8e12d 100644
> > --- a/drivers/acpi/resources/rsdump.c
> > +++ b/drivers/acpi/resources/rsdump.c
> > @@ -76,7 +76,7 @@ acpi_rs_dump_descriptor(void *resource, struct acpi_rsdump_info *table);
> >  
> >  #define ACPI_RSD_OFFSET(f)          (u8) ACPI_OFFSET (union acpi_resource_data,f)
> >  #define ACPI_PRT_OFFSET(f)          (u8) ACPI_OFFSET (struct acpi_pci_routing_table,f)
> > -#define ACPI_RSD_TABLE_SIZE(name)   (sizeof(name) / sizeof (struct acpi_rsdump_info))
> > +#define ACPI_RSD_TABLE_SIZE(name)   (ARRAY_SIZE(name))
> >   
> Any reason to not just replace ACPI_RSD_TABLE_SIZE with ARRAY_SIZE? Got 
> just 21 instances of it in the file (+ the define) and no more in the 
> rest of the tree.
I agree with that. Robert P. J. Day pointed out something similiar. For
a more verbose answer to that see the reply to his message.

> >  /*******************************************************************************
> >   *
> > diff --git a/drivers/acpi/tables/tbfadt.c b/drivers/acpi/tables/tbfadt.c
> > index 1285e91..4d59de2 100644
> > --- a/drivers/acpi/tables/tbfadt.c
> > +++ b/drivers/acpi/tables/tbfadt.c
> > @@ -104,7 +104,7 @@ static struct acpi_fadt_info fadt_info_table[] = {
> >  	 ACPI_FADT_OFFSET(gpe1_block_length), ACPI_FADT_SEPARATE_LENGTH}
> >  };
> >  
> > -#define ACPI_FADT_INFO_ENTRIES        (sizeof (fadt_info_table) / sizeof (struct acpi_fadt_info))
> > +#define ACPI_FADT_INFO_ENTRIES        (ARRAY_SIZE(fadt_info_table))
> >   
> Normally I think this is ok when it is just a "constant", since the name 
> of it may be more descriptive, but it is just used twice and I find 
> (imho) ARRAY_SIZE(fadt_info_table) easier to understand.
I totally agree with that. Here's a new patch that totally removes the ACPI_FADT_INFO_ENTRIES
macro. In contrast to the original patch, this one only affects tbfadt.c (I split this, because
there's another issue about rsdump.c pointed out by Robert P. J. Day. The other
changes will be in a reply to his message)

Tested by compilation.
Signed-off-by: Andi Drebes <lists-receive@programmierforen.de>
--

diff --git a/drivers/acpi/tables/tbfadt.c b/drivers/acpi/tables/tbfadt.c
index 1285e91..ba6d4a1 100644
--- a/drivers/acpi/tables/tbfadt.c
+++ b/drivers/acpi/tables/tbfadt.c
@@ -104,8 +104,6 @@ static struct acpi_fadt_info fadt_info_table[] = {
 	 ACPI_FADT_OFFSET(gpe1_block_length), ACPI_FADT_SEPARATE_LENGTH}
 };
 
-#define ACPI_FADT_INFO_ENTRIES        (sizeof (fadt_info_table) / sizeof (struct acpi_fadt_info))
-
 /*******************************************************************************
  *
  * FUNCTION:    acpi_tb_init_generic_address
@@ -295,7 +293,7 @@ static void acpi_tb_convert_fadt(void)
 	 * Expand the 32-bit V1.0 addresses to the 64-bit "X" generic address
 	 * structures as necessary.
 	 */
-	for (i = 0; i < ACPI_FADT_INFO_ENTRIES; i++) {
+	for (i = 0; i < ARRAY_SIZE(fadt_info_table); i++) {
 		target =
 		    ACPI_ADD_PTR(struct acpi_generic_address, &acpi_gbl_FADT,
 				 fadt_info_table[i].target);
@@ -392,7 +390,7 @@ static void acpi_tb_validate_fadt(void)
 
 	/* Examine all of the 64-bit extended address fields (X fields) */
 
-	for (i = 0; i < ACPI_FADT_INFO_ENTRIES; i++) {
+	for (i = 0; i < ARRAY_SIZE(fadt_info_table); i++) {
 
 		/* Generate pointers to the 32-bit and 64-bit addresses and get the length */
 

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations

[Len Brown]

 
> > Any reason to not just replace ACPI_RSD_TABLE_SIZE with ARRAY_SIZE?

Probably because ARRAY_SIZE doesn't exist in ACPICA, which is
where this code comes from...

When we change syntax in ACPICA files in Linux to make it more "beautiful",
then it creates more work for me -- as forever on, that syntax difference
must be manually compared to upstream ACPICA and Linux -- and that syntax
difference causes upstream patches to no longer apply and require
hand merging.

If you can license your patches to ACPICA files (drivers/acpi/*/*)
back to Intel to re-distribute under _both_ licenses at the top
of the file, then I'll be happy to apply them.
Otherwise, I'm really not excited about creating work
for no functional benefit.

thanks,
-Len
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations replaced with ARRAY_SIZE

[Len Brown]

 
> > Any reason to not just replace ACPI_RSD_TABLE_SIZE with ARRAY_SIZE?

Probably because ARRAY_SIZE doesn't exist in ACPICA, which is
where this code comes from...

When we change syntax in ACPICA files in Linux to make it more "beautiful",
then it creates more work for me -- as forever on, that syntax difference
must be manually compared to upstream ACPICA and Linux -- and that syntax
difference causes upstream patches to no longer apply and require
hand merging.

If you can license your patches to ACPICA files (drivers/acpi/*/*)
back to Intel to re-distribute under _both_ licenses at the top
of the file, then I'll be happy to apply them.
Otherwise, I'm really not excited about creating work
for no functional benefit.

thanks,
-Len

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations

[Christoph Hellwig]

On Wed, May 30, 2007 at 03:25:05PM -0400, Len Brown wrote:
>  
> > > Any reason to not just replace ACPI_RSD_TABLE_SIZE with ARRAY_SIZE?
> 
> Probably because ARRAY_SIZE doesn't exist in ACPICA, which is
> where this code comes from...
> 
> When we change syntax in ACPICA files in Linux to make it more "beautiful",
> then it creates more work for me -- as forever on, that syntax difference
> must be manually compared to upstream ACPICA and Linux -- and that syntax
> difference causes upstream patches to no longer apply and require
> hand merging.

Or we could stop that ACPCICA crap ASAP.  The acpi code not only looks
like crap because of that but it's buggy as hell now.

Intel is claiming how great linux is and how supportive is to it, but
can't even write proper code for their abomination of a firmware standard.
This is really more than dishonest.  Please take patches to get acpi into
shape and stop this complaining.

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations replaced with ARRAY_SIZE

[Christoph Hellwig]

On Wed, May 30, 2007 at 03:25:05PM -0400, Len Brown wrote:
>  
> > > Any reason to not just replace ACPI_RSD_TABLE_SIZE with ARRAY_SIZE?
> 
> Probably because ARRAY_SIZE doesn't exist in ACPICA, which is
> where this code comes from...
> 
> When we change syntax in ACPICA files in Linux to make it more "beautiful",
> then it creates more work for me -- as forever on, that syntax difference
> must be manually compared to upstream ACPICA and Linux -- and that syntax
> difference causes upstream patches to no longer apply and require
> hand merging.

Or we could stop that ACPCICA crap ASAP.  The acpi code not only looks
like crap because of that but it's buggy as hell now.

Intel is claiming how great linux is and how supportive is to it, but
can't even write proper code for their abomination of a firmware standard.
This is really more than dishonest.  Please take patches to get acpi into
shape and stop this complaining.

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations

[Pavel Machek]

Hi!

> > > > Any reason to not just replace ACPI_RSD_TABLE_SIZE with ARRAY_SIZE?
> > 
> > Probably because ARRAY_SIZE doesn't exist in ACPICA, which is
> > where this code comes from...
> > 
> > When we change syntax in ACPICA files in Linux to make it more "beautiful",
> > then it creates more work for me -- as forever on, that syntax difference
> > must be manually compared to upstream ACPICA and Linux -- and that syntax
> > difference causes upstream patches to no longer apply and require
> > hand merging.
> 
> Or we could stop that ACPCICA crap ASAP.  The acpi code not only looks
> like crap because of that but it's buggy as hell now.

+1.

Len, acpi subsystem is old enough to live by kernel standards, and
important enough that it should look&feel like a kernel code. It also
does not seem to change quickly, so merging patches should not be a
big deal.
							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations replaced with ARRAY_SIZE

[Pavel Machek]

Hi!

> > > > Any reason to not just replace ACPI_RSD_TABLE_SIZE with ARRAY_SIZE?
> > 
> > Probably because ARRAY_SIZE doesn't exist in ACPICA, which is
> > where this code comes from...
> > 
> > When we change syntax in ACPICA files in Linux to make it more "beautiful",
> > then it creates more work for me -- as forever on, that syntax difference
> > must be manually compared to upstream ACPICA and Linux -- and that syntax
> > difference causes upstream patches to no longer apply and require
> > hand merging.
> 
> Or we could stop that ACPCICA crap ASAP.  The acpi code not only looks
> like crap because of that but it's buggy as hell now.

+1.

Len, acpi subsystem is old enough to live by kernel standards, and
important enough that it should look&feel like a kernel code. It also
does not seem to change quickly, so merging patches should not be a
big deal.
							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations

[Bjorn Helgaas]

On Sunday 10 June 2007 04:57:12 am Pavel Machek wrote:
> > > > > Any reason to not just replace ACPI_RSD_TABLE_SIZE with ARRAY_SIZE?
> > > 
> > > Probably because ARRAY_SIZE doesn't exist in ACPICA, which is
> > > where this code comes from...
> > > 
> > > When we change syntax in ACPICA files in Linux to make it more "beautiful",
> > > then it creates more work for me -- as forever on, that syntax difference
> > > must be manually compared to upstream ACPICA and Linux -- and that syntax
> > > difference causes upstream patches to no longer apply and require
> > > hand merging.
> > 
> > Or we could stop that ACPCICA crap ASAP.  The acpi code not only looks
> > like crap because of that but it's buggy as hell now.
> 
> +1.
> 
> Len, acpi subsystem is old enough to live by kernel standards, and
> important enough that it should look&feel like a kernel code. It also
> does not seem to change quickly, so merging patches should not be a
> big deal.

I agree the ACPI CA is a nuisance.  But in this case, we're making
a mountain out of a molehill.  I suspect that if somebody spent the
15 minutes to make the ARRAY_SIZE patch work in both the Linux ACPI CA
and the generic Intel one and license it appropriately, Len would
happily apply the patch.
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations replaced with ARRAY_SIZE

[Bjorn Helgaas]

On Sunday 10 June 2007 04:57:12 am Pavel Machek wrote:
> > > > > Any reason to not just replace ACPI_RSD_TABLE_SIZE with ARRAY_SIZE?
> > > 
> > > Probably because ARRAY_SIZE doesn't exist in ACPICA, which is
> > > where this code comes from...
> > > 
> > > When we change syntax in ACPICA files in Linux to make it more "beautiful",
> > > then it creates more work for me -- as forever on, that syntax difference
> > > must be manually compared to upstream ACPICA and Linux -- and that syntax
> > > difference causes upstream patches to no longer apply and require
> > > hand merging.
> > 
> > Or we could stop that ACPCICA crap ASAP.  The acpi code not only looks
> > like crap because of that but it's buggy as hell now.
> 
> +1.
> 
> Len, acpi subsystem is old enough to live by kernel standards, and
> important enough that it should look&feel like a kernel code. It also
> does not seem to change quickly, so merging patches should not be a
> big deal.

I agree the ACPI CA is a nuisance.  But in this case, we're making
a mountain out of a molehill.  I suspect that if somebody spent the
15 minutes to make the ARRAY_SIZE patch work in both the Linux ACPI CA
and the generic Intel one and license it appropriately, Len would
happily apply the patch.

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations

[Andi Drebes]

First off: sorry for my late answer.

> I agree the ACPI CA is a nuisance.  But in this case, we're making
> a mountain out of a molehill.  I suspect that if somebody spent the
> 15 minutes to make the ARRAY_SIZE patch work in both the Linux ACPI CA
> and the generic Intel one and license it appropriately, Len would
> happily apply the patch.
I hope I got everything right. Here's what I understood so far:

The ACPI Subsystem in the kernel is derived from intel sources. I searched
the web for information about that and finally found

http://www.intel.com/technology/iapc/acpi/downloads.htm

Is that the right one? So what I would have to do in order to
"make the ARRAY_SIZE patch work in both the Linux ACPI CA
and the generic Intel one" is to modify those sources aswell,
send a patch to Intel and another one back to the
lkml.

Is that right? Or am I totally wrong?

	Cheers,
		Andi
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations replaced with ARRAY_SIZE

[Andi Drebes]

First off: sorry for my late answer.

> I agree the ACPI CA is a nuisance.  But in this case, we're making
> a mountain out of a molehill.  I suspect that if somebody spent the
> 15 minutes to make the ARRAY_SIZE patch work in both the Linux ACPI CA
> and the generic Intel one and license it appropriately, Len would
> happily apply the patch.
I hope I got everything right. Here's what I understood so far:

The ACPI Subsystem in the kernel is derived from intel sources. I searched
the web for information about that and finally found

http://www.intel.com/technology/iapc/acpi/downloads.htm

Is that the right one? So what I would have to do in order to
"make the ARRAY_SIZE patch work in both the Linux ACPI CA
and the generic Intel one" is to modify those sources aswell,
send a patch to Intel and another one back to the
lkml.

Is that right? Or am I totally wrong?

	Cheers,
		Andi

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations

[Bjorn Helgaas]

On Tuesday 12 June 2007 12:41:05 pm Andi Drebes wrote:
> First off: sorry for my late answer.
> 
> > I agree the ACPI CA is a nuisance.  But in this case, we're making
> > a mountain out of a molehill.  I suspect that if somebody spent the
> > 15 minutes to make the ARRAY_SIZE patch work in both the Linux ACPI CA
> > and the generic Intel one and license it appropriately, Len would
> > happily apply the patch.
> I hope I got everything right. Here's what I understood so far:
> 
> The ACPI Subsystem in the kernel is derived from intel sources. I searched
> the web for information about that and finally found
> 
> http://www.intel.com/technology/iapc/acpi/downloads.htm
> 
> Is that the right one? So what I would have to do in order to
> "make the ARRAY_SIZE patch work in both the Linux ACPI CA
> and the generic Intel one" is to modify those sources aswell,
> send a patch to Intel and another one back to the
> lkml.

You found the right one.  The ACPI CA in Linux is really the same
as the one from Intel.  Len integrates the Intel one into Linux
periodically.  He uses scripts or something to format it so it looks
more like Linux.

I think Len sometimes takes patches against the "Linux-ized" ACPI CA.
He probably has to apply those by hand to the Intel one, which is
the real "upstream" in this case.  So it might be a bit easier for
him if you generated a diff against the Intel version.  Then it
would show up in Linux, too, in the fullness of time.

In any event, you want to end up with one patch, not two.

You probably would have to add something like:

  #ifndef ARRAY_SIZE
  #define ARRAY_SIZE(x) ...
  #endif

somewhere in the ACPI CA header files.  That way it will work in
non-Linux OSes as well.

Bjorn
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations replaced with ARRAY_SIZE

[Bjorn Helgaas]

On Tuesday 12 June 2007 12:41:05 pm Andi Drebes wrote:
> First off: sorry for my late answer.
> 
> > I agree the ACPI CA is a nuisance.  But in this case, we're making
> > a mountain out of a molehill.  I suspect that if somebody spent the
> > 15 minutes to make the ARRAY_SIZE patch work in both the Linux ACPI CA
> > and the generic Intel one and license it appropriately, Len would
> > happily apply the patch.
> I hope I got everything right. Here's what I understood so far:
> 
> The ACPI Subsystem in the kernel is derived from intel sources. I searched
> the web for information about that and finally found
> 
> http://www.intel.com/technology/iapc/acpi/downloads.htm
> 
> Is that the right one? So what I would have to do in order to
> "make the ARRAY_SIZE patch work in both the Linux ACPI CA
> and the generic Intel one" is to modify those sources aswell,
> send a patch to Intel and another one back to the
> lkml.

You found the right one.  The ACPI CA in Linux is really the same
as the one from Intel.  Len integrates the Intel one into Linux
periodically.  He uses scripts or something to format it so it looks
more like Linux.

I think Len sometimes takes patches against the "Linux-ized" ACPI CA.
He probably has to apply those by hand to the Intel one, which is
the real "upstream" in this case.  So it might be a bit easier for
him if you generated a diff against the Intel version.  Then it
would show up in Linux, too, in the fullness of time.

In any event, you want to end up with one patch, not two.

You probably would have to add something like:

  #ifndef ARRAY_SIZE
  #define ARRAY_SIZE(x) ...
  #endif

somewhere in the ACPI CA header files.  That way it will work in
non-Linux OSes as well.

Bjorn

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations

[Andi Drebes]

<snip>
> You found the right one.  The ACPI CA in Linux is really the same
> as the one from Intel.  Len integrates the Intel one into Linux
> periodically.  He uses scripts or something to format it so it looks
> more like Linux.
Ok. That's what I thought so far.

> I think Len sometimes takes patches against the "Linux-ized" ACPI CA.
> He probably has to apply those by hand to the Intel one, which is
> the real "upstream" in this case.  So it might be a bit easier for
> him if you generated a diff against the Intel version. 
OK. I will do this in the next time.

> In any event, you want to end up with one patch, not two.
Ok. I thought of two patches because I didn't know how strong
the sources from the  ACPI CA are modified.

> You probably would have to add something like:
> 
>   #ifndef ARRAY_SIZE
>   #define ARRAY_SIZE(x) ...
>   #endif
> 
> somewhere in the ACPI CA header files.  That way it will work in
> non-Linux OSes as well.

Thanks so far,
	Andi
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations replaced with ARRAY_SIZE

[Andi Drebes]

<snip>
> You found the right one.  The ACPI CA in Linux is really the same
> as the one from Intel.  Len integrates the Intel one into Linux
> periodically.  He uses scripts or something to format it so it looks
> more like Linux.
Ok. That's what I thought so far.

> I think Len sometimes takes patches against the "Linux-ized" ACPI CA.
> He probably has to apply those by hand to the Intel one, which is
> the real "upstream" in this case.  So it might be a bit easier for
> him if you generated a diff against the Intel version. 
OK. I will do this in the next time.

> In any event, you want to end up with one patch, not two.
Ok. I thought of two patches because I didn't know how strong
the sources from the  ACPI CA are modified.

> You probably would have to add something like:
> 
>   #ifndef ARRAY_SIZE
>   #define ARRAY_SIZE(x) ...
>   #endif
> 
> somewhere in the ACPI CA header files.  That way it will work in
> non-Linux OSes as well.

Thanks so far,
	Andi

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations

[Andi Drebes]

<snip>
> You found the right one.  The ACPI CA in Linux is really the same
> as the one from Intel.
Hmmm. The file that I downloaded recently is acpica-unix-20061109. It seems
to be quite old. There are some differences between the contents
of that archive and the acpi files in the kernel. I hope I really got the right version.

> So it might be a bit easier for
> him if you generated a diff against the Intel version.  Then it
> would show up in Linux, too, in the fullness of time.
Ok. The new patch can be found in this message.
I tried to figure out where to put the definition of the ARRAY_SIZE macro.
I searched a little bit in the include directory, but I couldn't find a place
that satisfied me to a hundred percent. I chose acpi.h, because this
seems to be file that is most general. The problem is, that the file
only consists of lines that include other header files and the newly added
definition for ARRAY_SIZE breaks this consistency in some way. Perhaps
Len could give me a hint where to put it.

Ok, here's the new patch:

Diffed against the intel acpica implementation "acpica-unix-20061109".
Tested by compilation.

Signed-off-by: Andi Drebes <lists-receive@programmierforen.de>
---
diff --git a/include/acpi.h b/include/acpi.h
index ddaeec4..9de744c 100644
--- a/include/acpi.h
+++ b/include/acpi.h
@@ -139,5 +139,11 @@
 #include "achware.h"            /* Hardware defines and interfaces */
 #include "acutils.h"            /* Utility interfaces */
 
+#ifndef ARRAY_SIZE
+/*
+ * ARRAY_SIZE calculates the size of a statically initialized array
+ */
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
+#endif
 
 #endif /* __ACPI_H__ */
diff --git a/resources/rsdump.c b/resources/rsdump.c
index 1f90cf7..4eea301 100644
--- a/resources/rsdump.c
+++ b/resources/rsdump.c
@@ -188,8 +188,6 @@ AcpiRsDumpDescriptor (
 
 #define ACPI_RSD_OFFSET(f)          (UINT8) ACPI_OFFSET (ACPI_RESOURCE_DATA,f)
 #define ACPI_PRT_OFFSET(f)          (UINT8) ACPI_OFFSET (ACPI_PCI_ROUTING_TABLE,f)
-#define ACPI_RSD_TABLE_SIZE(name)   (sizeof(name) / sizeof (ACPI_RSDUMP_INFO))
-
 
 /*******************************************************************************
  *
@@ -202,7 +200,7 @@ AcpiRsDumpDescriptor (
 
 ACPI_RSDUMP_INFO        AcpiRsDumpIrq[6]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpIrq),                "IRQ",                      NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpIrq),                         "IRQ",                      NULL},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Irq.Triggering),                   "Triggering",               AcpiGbl_HeDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Irq.Polarity),                     "Polarity",                 AcpiGbl_LlDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Irq.Sharable),                     "Sharing",                  AcpiGbl_ShrDecode},
@@ -212,7 +210,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpIrq[6]  
 ACPI_RSDUMP_INFO        AcpiRsDumpDma[6]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpDma),                "DMA",                      NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpDma),                         "DMA",                      NULL},
     {ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET (Dma.Type),                         "Speed",                    AcpiGbl_TypDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Dma.BusMaster),                    "Mastering",                AcpiGbl_BmDecode},
     {ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET (Dma.Transfer),                     "Transfer Type",            AcpiGbl_SizDecode},
@@ -222,19 +220,19 @@ ACPI_RSDUMP_INFO        AcpiRsDumpDma[6]  
 ACPI_RSDUMP_INFO        AcpiRsDumpStartDpf[3]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpStartDpf),           "Start-Dependent-Functions",NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpStartDpf),                    "Start-Dependent-Functions",NULL},
     {ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET (StartDpf.CompatibilityPriority),   "Compatibility Priority",   AcpiGbl_ConfigDecode},
     {ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET (StartDpf.PerformanceRobustness),   "Performance/Robustness",   AcpiGbl_ConfigDecode}
 };
 
 ACPI_RSDUMP_INFO        AcpiRsDumpEndDpf[1]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpEndDpf),             "End-Dependent-Functions",  NULL}
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpEndDpf),                      "End-Dependent-Functions",  NULL}
 };
 
 ACPI_RSDUMP_INFO        AcpiRsDumpIo[6]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpIo),                 "I/O",                      NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpIo),                          "I/O",                      NULL},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Io.IoDecode),                      "Address Decoding",         AcpiGbl_IoDecode},
     {ACPI_RSD_UINT16,   ACPI_RSD_OFFSET (Io.Minimum),                       "Address Minimum",          NULL},
     {ACPI_RSD_UINT16,   ACPI_RSD_OFFSET (Io.Maximum),                       "Address Maximum",          NULL},
@@ -244,26 +242,26 @@ ACPI_RSDUMP_INFO        AcpiRsDumpIo[6]  
 ACPI_RSDUMP_INFO        AcpiRsDumpFixedIo[3]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpFixedIo),            "Fixed I/O",                NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpFixedIo),                     "Fixed I/O",                NULL},
     {ACPI_RSD_UINT16,   ACPI_RSD_OFFSET (FixedIo.Address),                  "Address",                  NULL},
     {ACPI_RSD_UINT8,    ACPI_RSD_OFFSET (FixedIo.AddressLength),            "Address Length",           NULL}
 };
 
 ACPI_RSDUMP_INFO        AcpiRsDumpVendor[3]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpVendor),             "Vendor Specific",          NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpVendor),                      "Vendor Specific",          NULL},
     {ACPI_RSD_UINT16,   ACPI_RSD_OFFSET (Vendor.ByteLength),                "Length",                   NULL},
     {ACPI_RSD_LONGLIST, ACPI_RSD_OFFSET (Vendor.ByteData[0]),               "Vendor Data",              NULL}
 };
 
 ACPI_RSDUMP_INFO        AcpiRsDumpEndTag[1]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpEndTag),             "EndTag",                   NULL}
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpEndTag),                      "EndTag",                   NULL}
 };
 
 ACPI_RSDUMP_INFO        AcpiRsDumpMemory24[6]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpMemory24),           "24-Bit Memory Range",      NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpMemory24),                    "24-Bit Memory Range",      NULL},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Memory24.WriteProtect),            "Write Protect",            AcpiGbl_RwDecode},
     {ACPI_RSD_UINT16,   ACPI_RSD_OFFSET (Memory24.Minimum),                 "Address Minimum",          NULL},
     {ACPI_RSD_UINT16,   ACPI_RSD_OFFSET (Memory24.Maximum),                 "Address Maximum",          NULL},
@@ -273,7 +271,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpMemory24[6]  
 ACPI_RSDUMP_INFO        AcpiRsDumpMemory32[6]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpMemory32),           "32-Bit Memory Range",      NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpMemory32),                    "32-Bit Memory Range",      NULL},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Memory32.WriteProtect),            "Write Protect",            AcpiGbl_RwDecode},
     {ACPI_RSD_UINT32,   ACPI_RSD_OFFSET (Memory32.Minimum),                 "Address Minimum",          NULL},
     {ACPI_RSD_UINT32,   ACPI_RSD_OFFSET (Memory32.Maximum),                 "Address Maximum",          NULL},
@@ -283,7 +281,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpMemory32[6]  
 ACPI_RSDUMP_INFO        AcpiRsDumpFixedMemory32[4]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpFixedMemory32),      "32-Bit Fixed Memory Range",NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpFixedMemory32),               "32-Bit Fixed Memory Range",NULL},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (FixedMemory32.WriteProtect),       "Write Protect",            AcpiGbl_RwDecode},
     {ACPI_RSD_UINT32,   ACPI_RSD_OFFSET (FixedMemory32.Address),            "Address",                  NULL},
     {ACPI_RSD_UINT32,   ACPI_RSD_OFFSET (FixedMemory32.AddressLength),      "Address Length",           NULL}
@@ -291,7 +289,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpFixedMemory32[4]  
 ACPI_RSDUMP_INFO        AcpiRsDumpAddress16[8]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpAddress16),          "16-Bit WORD Address Space",NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpAddress16),                   "16-Bit WORD Address Space",NULL},
     {ACPI_RSD_ADDRESS,  0,                                                  NULL,                       NULL},
     {ACPI_RSD_UINT16,   ACPI_RSD_OFFSET (Address16.Granularity),            "Granularity",              NULL},
     {ACPI_RSD_UINT16,   ACPI_RSD_OFFSET (Address16.Minimum),                "Address Minimum",          NULL},
@@ -303,7 +301,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpAddress16[8]  
 ACPI_RSDUMP_INFO        AcpiRsDumpAddress32[8]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpAddress32),         "32-Bit DWORD Address Space", NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpAddress32),                   "32-Bit DWORD Address Space", NULL},
     {ACPI_RSD_ADDRESS,  0,                                                  NULL,                       NULL},
     {ACPI_RSD_UINT32,   ACPI_RSD_OFFSET (Address32.Granularity),            "Granularity",              NULL},
     {ACPI_RSD_UINT32,   ACPI_RSD_OFFSET (Address32.Minimum),                "Address Minimum",          NULL},
@@ -315,7 +313,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpAddress32[8]  
 ACPI_RSDUMP_INFO        AcpiRsDumpAddress64[8]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpAddress64),          "64-Bit QWORD Address Space", NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpAddress64),                   "64-Bit QWORD Address Space", NULL},
     {ACPI_RSD_ADDRESS,  0,                                                  NULL,                       NULL},
     {ACPI_RSD_UINT64,   ACPI_RSD_OFFSET (Address64.Granularity),            "Granularity",              NULL},
     {ACPI_RSD_UINT64,   ACPI_RSD_OFFSET (Address64.Minimum),                "Address Minimum",          NULL},
@@ -327,7 +325,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpAddress64[8]  
 ACPI_RSDUMP_INFO        AcpiRsDumpExtAddress64[8]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpExtAddress64),       "64-Bit Extended Address Space", NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpExtAddress64),                "64-Bit Extended Address Space", NULL},
     {ACPI_RSD_ADDRESS,  0,                                                  NULL,                       NULL},
     {ACPI_RSD_UINT64,   ACPI_RSD_OFFSET (ExtAddress64.Granularity),         "Granularity",              NULL},
     {ACPI_RSD_UINT64,   ACPI_RSD_OFFSET (ExtAddress64.Minimum),             "Address Minimum",          NULL},
@@ -339,7 +337,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpExtAddress64[8]  
 ACPI_RSDUMP_INFO        AcpiRsDumpExtIrq[8]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpExtIrq),             "Extended IRQ",             NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpExtIrq),                      "Extended IRQ",             NULL},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (ExtendedIrq.ProducerConsumer),     "Type",                     AcpiGbl_ConsumeDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (ExtendedIrq.Triggering),           "Triggering",               AcpiGbl_HeDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (ExtendedIrq.Polarity),             "Polarity",                 AcpiGbl_LlDecode},
@@ -351,7 +349,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpExtIrq[8]  
 ACPI_RSDUMP_INFO        AcpiRsDumpGenericReg[6]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpGenericReg),         "Generic Register",         NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpGenericReg),                  "Generic Register",         NULL},
     {ACPI_RSD_UINT8,    ACPI_RSD_OFFSET (GenericReg.SpaceId),               "Space ID",                 NULL},
     {ACPI_RSD_UINT8,    ACPI_RSD_OFFSET (GenericReg.BitWidth),              "Bit Width",                NULL},
     {ACPI_RSD_UINT8,    ACPI_RSD_OFFSET (GenericReg.BitOffset),             "Bit Offset",               NULL},
@@ -365,7 +363,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpGenericReg[6]   */
 static ACPI_RSDUMP_INFO AcpiRsDumpGeneralFlags[5]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpGeneralFlags),       NULL,                       NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpGeneralFlags),                NULL,                       NULL},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Address.ProducerConsumer),         "Consumer/Producer",        AcpiGbl_ConsumeDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Address.Decode),                   "Address Decode",           AcpiGbl_DecDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Address.MinAddressFixed),          "Min Relocatability",       AcpiGbl_MinDecode},
@@ -374,7 +372,7 @@ static ACPI_RSDUMP_INFO AcpiRsDumpGeneralFlags[5]  
 static ACPI_RSDUMP_INFO AcpiRsDumpMemoryFlags[5]  {
-    {ACPI_RSD_LITERAL,  ACPI_RSD_TABLE_SIZE (AcpiRsDumpMemoryFlags),        "Resource Type",            (void *) "Memory Range"},
+    {ACPI_RSD_LITERAL,  ARRAY_SIZE (AcpiRsDumpMemoryFlags),                 "Resource Type",            (void *) "Memory Range"},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Address.Info.Mem.WriteProtect),    "Write Protect",            AcpiGbl_RwDecode},
     {ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET (Address.Info.Mem.Caching),         "Caching",                  AcpiGbl_MemDecode},
     {ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET (Address.Info.Mem.RangeType),       "Range Type",               AcpiGbl_MtpDecode},
@@ -383,7 +381,7 @@ static ACPI_RSDUMP_INFO AcpiRsDumpMemoryFlags[5]  
 static ACPI_RSDUMP_INFO AcpiRsDumpIoFlags[4]  {
-    {ACPI_RSD_LITERAL,  ACPI_RSD_TABLE_SIZE (AcpiRsDumpIoFlags),            "Resource Type",            (void *) "I/O Range"},
+    {ACPI_RSD_LITERAL,  ARRAY_SIZE (AcpiRsDumpIoFlags),                     "Resource Type",            (void *) "I/O Range"},
     {ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET (Address.Info.Io.RangeType),        "Range Type",               AcpiGbl_RngDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Address.Info.Io.Translation),      "Translation",              AcpiGbl_TtpDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Address.Info.Io.TranslationType),  "Translation Type",         AcpiGbl_TrsDecode}
@@ -395,7 +393,7 @@ static ACPI_RSDUMP_INFO AcpiRsDumpIoFlags[4]   */
 static ACPI_RSDUMP_INFO   AcpiRsDumpPrt[5]  {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpPrt),                NULL,                       NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpPrt),                         NULL,                       NULL},
     {ACPI_RSD_UINT64,   ACPI_PRT_OFFSET (Address),                          "Address",                  NULL},
     {ACPI_RSD_UINT32,   ACPI_PRT_OFFSET (Pin),                              "Pin",                      NULL},
     {ACPI_RSD_STRING,   ACPI_PRT_OFFSET (Source[0]),                        "Source",                   NULL},
diff --git a/tables/tbfadt.c b/tables/tbfadt.c
index a1afd6d..04f2fac 100644
--- a/tables/tbfadt.c
+++ b/tables/tbfadt.c
@@ -189,9 +189,6 @@ static ACPI_FADT_INFO     FadtInfoTable[]                              ACPI_FADT_OFFSET (Gpe1BlockLength),     ACPI_FADT_SEPARATE_LENGTH}
 };
 
-#define ACPI_FADT_INFO_ENTRIES        (sizeof (FadtInfoTable) / sizeof (ACPI_FADT_INFO))
-
-
 /*******************************************************************************
  *
  * FUNCTION:    AcpiTbInitGenericAddress
@@ -398,7 +395,7 @@ AcpiTbConvertFadt (
      * Expand the 32-bit V1.0 addresses to the 64-bit "X" generic address
      * structures as necessary.
      */
-    for (i = 0; i < ACPI_FADT_INFO_ENTRIES; i++)
+    for (i = 0; i < ARRAY_SIZE(FadtInfoTable); i++)
     {
         Target = ACPI_ADD_PTR (
             ACPI_GENERIC_ADDRESS, &AcpiGbl_FADT, FadtInfoTable[i].Target);
@@ -471,7 +468,7 @@ AcpiTbValidateFadt (
 
     /* Examine all of the 64-bit extended address fields (X fields) */
 
-    for (i = 0; i < ACPI_FADT_INFO_ENTRIES; i++)
+    for (i = 0; i < ARRAY_SIZE(FadtInfoTable); i++)
     {
         /* Generate pointers to the 32-bit and 64-bit addresses and get the length */
 
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations replaced with ARRAY_SIZE

[Andi Drebes]

<snip>
> You found the right one.  The ACPI CA in Linux is really the same
> as the one from Intel.
Hmmm. The file that I downloaded recently is acpica-unix-20061109. It seems
to be quite old. There are some differences between the contents
of that archive and the acpi files in the kernel. I hope I really got the right version.

> So it might be a bit easier for
> him if you generated a diff against the Intel version.  Then it
> would show up in Linux, too, in the fullness of time.
Ok. The new patch can be found in this message.
I tried to figure out where to put the definition of the ARRAY_SIZE macro.
I searched a little bit in the include directory, but I couldn't find a place
that satisfied me to a hundred percent. I chose acpi.h, because this
seems to be file that is most general. The problem is, that the file
only consists of lines that include other header files and the newly added
definition for ARRAY_SIZE breaks this consistency in some way. Perhaps
Len could give me a hint where to put it.

Ok, here's the new patch:

Diffed against the intel acpica implementation "acpica-unix-20061109".
Tested by compilation.

Signed-off-by: Andi Drebes <lists-receive@programmierforen.de>
---
diff --git a/include/acpi.h b/include/acpi.h
index ddaeec4..9de744c 100644
--- a/include/acpi.h
+++ b/include/acpi.h
@@ -139,5 +139,11 @@
 #include "achware.h"            /* Hardware defines and interfaces */
 #include "acutils.h"            /* Utility interfaces */
 
+#ifndef ARRAY_SIZE
+/*
+ * ARRAY_SIZE calculates the size of a statically initialized array
+ */
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
+#endif
 
 #endif /* __ACPI_H__ */
diff --git a/resources/rsdump.c b/resources/rsdump.c
index 1f90cf7..4eea301 100644
--- a/resources/rsdump.c
+++ b/resources/rsdump.c
@@ -188,8 +188,6 @@ AcpiRsDumpDescriptor (
 
 #define ACPI_RSD_OFFSET(f)          (UINT8) ACPI_OFFSET (ACPI_RESOURCE_DATA,f)
 #define ACPI_PRT_OFFSET(f)          (UINT8) ACPI_OFFSET (ACPI_PCI_ROUTING_TABLE,f)
-#define ACPI_RSD_TABLE_SIZE(name)   (sizeof(name) / sizeof (ACPI_RSDUMP_INFO))
-
 
 /*******************************************************************************
  *
@@ -202,7 +200,7 @@ AcpiRsDumpDescriptor (
 
 ACPI_RSDUMP_INFO        AcpiRsDumpIrq[6] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpIrq),                "IRQ",                      NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpIrq),                         "IRQ",                      NULL},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Irq.Triggering),                   "Triggering",               AcpiGbl_HeDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Irq.Polarity),                     "Polarity",                 AcpiGbl_LlDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Irq.Sharable),                     "Sharing",                  AcpiGbl_ShrDecode},
@@ -212,7 +210,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpIrq[6] =
 
 ACPI_RSDUMP_INFO        AcpiRsDumpDma[6] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpDma),                "DMA",                      NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpDma),                         "DMA",                      NULL},
     {ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET (Dma.Type),                         "Speed",                    AcpiGbl_TypDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Dma.BusMaster),                    "Mastering",                AcpiGbl_BmDecode},
     {ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET (Dma.Transfer),                     "Transfer Type",            AcpiGbl_SizDecode},
@@ -222,19 +220,19 @@ ACPI_RSDUMP_INFO        AcpiRsDumpDma[6] =
 
 ACPI_RSDUMP_INFO        AcpiRsDumpStartDpf[3] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpStartDpf),           "Start-Dependent-Functions",NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpStartDpf),                    "Start-Dependent-Functions",NULL},
     {ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET (StartDpf.CompatibilityPriority),   "Compatibility Priority",   AcpiGbl_ConfigDecode},
     {ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET (StartDpf.PerformanceRobustness),   "Performance/Robustness",   AcpiGbl_ConfigDecode}
 };
 
 ACPI_RSDUMP_INFO        AcpiRsDumpEndDpf[1] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpEndDpf),             "End-Dependent-Functions",  NULL}
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpEndDpf),                      "End-Dependent-Functions",  NULL}
 };
 
 ACPI_RSDUMP_INFO        AcpiRsDumpIo[6] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpIo),                 "I/O",                      NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpIo),                          "I/O",                      NULL},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Io.IoDecode),                      "Address Decoding",         AcpiGbl_IoDecode},
     {ACPI_RSD_UINT16,   ACPI_RSD_OFFSET (Io.Minimum),                       "Address Minimum",          NULL},
     {ACPI_RSD_UINT16,   ACPI_RSD_OFFSET (Io.Maximum),                       "Address Maximum",          NULL},
@@ -244,26 +242,26 @@ ACPI_RSDUMP_INFO        AcpiRsDumpIo[6] =
 
 ACPI_RSDUMP_INFO        AcpiRsDumpFixedIo[3] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpFixedIo),            "Fixed I/O",                NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpFixedIo),                     "Fixed I/O",                NULL},
     {ACPI_RSD_UINT16,   ACPI_RSD_OFFSET (FixedIo.Address),                  "Address",                  NULL},
     {ACPI_RSD_UINT8,    ACPI_RSD_OFFSET (FixedIo.AddressLength),            "Address Length",           NULL}
 };
 
 ACPI_RSDUMP_INFO        AcpiRsDumpVendor[3] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpVendor),             "Vendor Specific",          NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpVendor),                      "Vendor Specific",          NULL},
     {ACPI_RSD_UINT16,   ACPI_RSD_OFFSET (Vendor.ByteLength),                "Length",                   NULL},
     {ACPI_RSD_LONGLIST, ACPI_RSD_OFFSET (Vendor.ByteData[0]),               "Vendor Data",              NULL}
 };
 
 ACPI_RSDUMP_INFO        AcpiRsDumpEndTag[1] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpEndTag),             "EndTag",                   NULL}
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpEndTag),                      "EndTag",                   NULL}
 };
 
 ACPI_RSDUMP_INFO        AcpiRsDumpMemory24[6] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpMemory24),           "24-Bit Memory Range",      NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpMemory24),                    "24-Bit Memory Range",      NULL},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Memory24.WriteProtect),            "Write Protect",            AcpiGbl_RwDecode},
     {ACPI_RSD_UINT16,   ACPI_RSD_OFFSET (Memory24.Minimum),                 "Address Minimum",          NULL},
     {ACPI_RSD_UINT16,   ACPI_RSD_OFFSET (Memory24.Maximum),                 "Address Maximum",          NULL},
@@ -273,7 +271,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpMemory24[6] =
 
 ACPI_RSDUMP_INFO        AcpiRsDumpMemory32[6] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpMemory32),           "32-Bit Memory Range",      NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpMemory32),                    "32-Bit Memory Range",      NULL},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Memory32.WriteProtect),            "Write Protect",            AcpiGbl_RwDecode},
     {ACPI_RSD_UINT32,   ACPI_RSD_OFFSET (Memory32.Minimum),                 "Address Minimum",          NULL},
     {ACPI_RSD_UINT32,   ACPI_RSD_OFFSET (Memory32.Maximum),                 "Address Maximum",          NULL},
@@ -283,7 +281,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpMemory32[6] =
 
 ACPI_RSDUMP_INFO        AcpiRsDumpFixedMemory32[4] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpFixedMemory32),      "32-Bit Fixed Memory Range",NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpFixedMemory32),               "32-Bit Fixed Memory Range",NULL},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (FixedMemory32.WriteProtect),       "Write Protect",            AcpiGbl_RwDecode},
     {ACPI_RSD_UINT32,   ACPI_RSD_OFFSET (FixedMemory32.Address),            "Address",                  NULL},
     {ACPI_RSD_UINT32,   ACPI_RSD_OFFSET (FixedMemory32.AddressLength),      "Address Length",           NULL}
@@ -291,7 +289,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpFixedMemory32[4] =
 
 ACPI_RSDUMP_INFO        AcpiRsDumpAddress16[8] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpAddress16),          "16-Bit WORD Address Space",NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpAddress16),                   "16-Bit WORD Address Space",NULL},
     {ACPI_RSD_ADDRESS,  0,                                                  NULL,                       NULL},
     {ACPI_RSD_UINT16,   ACPI_RSD_OFFSET (Address16.Granularity),            "Granularity",              NULL},
     {ACPI_RSD_UINT16,   ACPI_RSD_OFFSET (Address16.Minimum),                "Address Minimum",          NULL},
@@ -303,7 +301,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpAddress16[8] =
 
 ACPI_RSDUMP_INFO        AcpiRsDumpAddress32[8] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpAddress32),         "32-Bit DWORD Address Space", NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpAddress32),                   "32-Bit DWORD Address Space", NULL},
     {ACPI_RSD_ADDRESS,  0,                                                  NULL,                       NULL},
     {ACPI_RSD_UINT32,   ACPI_RSD_OFFSET (Address32.Granularity),            "Granularity",              NULL},
     {ACPI_RSD_UINT32,   ACPI_RSD_OFFSET (Address32.Minimum),                "Address Minimum",          NULL},
@@ -315,7 +313,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpAddress32[8] =
 
 ACPI_RSDUMP_INFO        AcpiRsDumpAddress64[8] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpAddress64),          "64-Bit QWORD Address Space", NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpAddress64),                   "64-Bit QWORD Address Space", NULL},
     {ACPI_RSD_ADDRESS,  0,                                                  NULL,                       NULL},
     {ACPI_RSD_UINT64,   ACPI_RSD_OFFSET (Address64.Granularity),            "Granularity",              NULL},
     {ACPI_RSD_UINT64,   ACPI_RSD_OFFSET (Address64.Minimum),                "Address Minimum",          NULL},
@@ -327,7 +325,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpAddress64[8] =
 
 ACPI_RSDUMP_INFO        AcpiRsDumpExtAddress64[8] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpExtAddress64),       "64-Bit Extended Address Space", NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpExtAddress64),                "64-Bit Extended Address Space", NULL},
     {ACPI_RSD_ADDRESS,  0,                                                  NULL,                       NULL},
     {ACPI_RSD_UINT64,   ACPI_RSD_OFFSET (ExtAddress64.Granularity),         "Granularity",              NULL},
     {ACPI_RSD_UINT64,   ACPI_RSD_OFFSET (ExtAddress64.Minimum),             "Address Minimum",          NULL},
@@ -339,7 +337,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpExtAddress64[8] =
 
 ACPI_RSDUMP_INFO        AcpiRsDumpExtIrq[8] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpExtIrq),             "Extended IRQ",             NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpExtIrq),                      "Extended IRQ",             NULL},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (ExtendedIrq.ProducerConsumer),     "Type",                     AcpiGbl_ConsumeDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (ExtendedIrq.Triggering),           "Triggering",               AcpiGbl_HeDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (ExtendedIrq.Polarity),             "Polarity",                 AcpiGbl_LlDecode},
@@ -351,7 +349,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpExtIrq[8] =
 
 ACPI_RSDUMP_INFO        AcpiRsDumpGenericReg[6] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpGenericReg),         "Generic Register",         NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpGenericReg),                  "Generic Register",         NULL},
     {ACPI_RSD_UINT8,    ACPI_RSD_OFFSET (GenericReg.SpaceId),               "Space ID",                 NULL},
     {ACPI_RSD_UINT8,    ACPI_RSD_OFFSET (GenericReg.BitWidth),              "Bit Width",                NULL},
     {ACPI_RSD_UINT8,    ACPI_RSD_OFFSET (GenericReg.BitOffset),             "Bit Offset",               NULL},
@@ -365,7 +363,7 @@ ACPI_RSDUMP_INFO        AcpiRsDumpGenericReg[6] =
  */
 static ACPI_RSDUMP_INFO AcpiRsDumpGeneralFlags[5] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpGeneralFlags),       NULL,                       NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpGeneralFlags),                NULL,                       NULL},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Address.ProducerConsumer),         "Consumer/Producer",        AcpiGbl_ConsumeDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Address.Decode),                   "Address Decode",           AcpiGbl_DecDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Address.MinAddressFixed),          "Min Relocatability",       AcpiGbl_MinDecode},
@@ -374,7 +372,7 @@ static ACPI_RSDUMP_INFO AcpiRsDumpGeneralFlags[5] =
 
 static ACPI_RSDUMP_INFO AcpiRsDumpMemoryFlags[5] =
 {
-    {ACPI_RSD_LITERAL,  ACPI_RSD_TABLE_SIZE (AcpiRsDumpMemoryFlags),        "Resource Type",            (void *) "Memory Range"},
+    {ACPI_RSD_LITERAL,  ARRAY_SIZE (AcpiRsDumpMemoryFlags),                 "Resource Type",            (void *) "Memory Range"},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Address.Info.Mem.WriteProtect),    "Write Protect",            AcpiGbl_RwDecode},
     {ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET (Address.Info.Mem.Caching),         "Caching",                  AcpiGbl_MemDecode},
     {ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET (Address.Info.Mem.RangeType),       "Range Type",               AcpiGbl_MtpDecode},
@@ -383,7 +381,7 @@ static ACPI_RSDUMP_INFO AcpiRsDumpMemoryFlags[5] =
 
 static ACPI_RSDUMP_INFO AcpiRsDumpIoFlags[4] =
 {
-    {ACPI_RSD_LITERAL,  ACPI_RSD_TABLE_SIZE (AcpiRsDumpIoFlags),            "Resource Type",            (void *) "I/O Range"},
+    {ACPI_RSD_LITERAL,  ARRAY_SIZE (AcpiRsDumpIoFlags),                     "Resource Type",            (void *) "I/O Range"},
     {ACPI_RSD_2BITFLAG, ACPI_RSD_OFFSET (Address.Info.Io.RangeType),        "Range Type",               AcpiGbl_RngDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Address.Info.Io.Translation),      "Translation",              AcpiGbl_TtpDecode},
     {ACPI_RSD_1BITFLAG, ACPI_RSD_OFFSET (Address.Info.Io.TranslationType),  "Translation Type",         AcpiGbl_TrsDecode}
@@ -395,7 +393,7 @@ static ACPI_RSDUMP_INFO AcpiRsDumpIoFlags[4] =
  */
 static ACPI_RSDUMP_INFO   AcpiRsDumpPrt[5] =
 {
-    {ACPI_RSD_TITLE,    ACPI_RSD_TABLE_SIZE (AcpiRsDumpPrt),                NULL,                       NULL},
+    {ACPI_RSD_TITLE,    ARRAY_SIZE (AcpiRsDumpPrt),                         NULL,                       NULL},
     {ACPI_RSD_UINT64,   ACPI_PRT_OFFSET (Address),                          "Address",                  NULL},
     {ACPI_RSD_UINT32,   ACPI_PRT_OFFSET (Pin),                              "Pin",                      NULL},
     {ACPI_RSD_STRING,   ACPI_PRT_OFFSET (Source[0]),                        "Source",                   NULL},
diff --git a/tables/tbfadt.c b/tables/tbfadt.c
index a1afd6d..04f2fac 100644
--- a/tables/tbfadt.c
+++ b/tables/tbfadt.c
@@ -189,9 +189,6 @@ static ACPI_FADT_INFO     FadtInfoTable[] =
                             ACPI_FADT_OFFSET (Gpe1BlockLength),     ACPI_FADT_SEPARATE_LENGTH}
 };
 
-#define ACPI_FADT_INFO_ENTRIES        (sizeof (FadtInfoTable) / sizeof (ACPI_FADT_INFO))
-
-
 /*******************************************************************************
  *
  * FUNCTION:    AcpiTbInitGenericAddress
@@ -398,7 +395,7 @@ AcpiTbConvertFadt (
      * Expand the 32-bit V1.0 addresses to the 64-bit "X" generic address
      * structures as necessary.
      */
-    for (i = 0; i < ACPI_FADT_INFO_ENTRIES; i++)
+    for (i = 0; i < ARRAY_SIZE(FadtInfoTable); i++)
     {
         Target = ACPI_ADD_PTR (
             ACPI_GENERIC_ADDRESS, &AcpiGbl_FADT, FadtInfoTable[i].Target);
@@ -471,7 +468,7 @@ AcpiTbValidateFadt (
 
     /* Examine all of the 64-bit extended address fields (X fields) */
 
-    for (i = 0; i < ACPI_FADT_INFO_ENTRIES; i++)
+    for (i = 0; i < ARRAY_SIZE(FadtInfoTable); i++)
     {
         /* Generate pointers to the 32-bit and 64-bit addresses and get the length */