On Mon, Dec 17, 2018 at 06:36:05PM +0100, Ingo Molnar wrote: > >* Ingo Molnar <mi...@kernel.org> wrote: > >> > + if (!(efi_guidcmp(guid, ACPI_TABLE_GUID))) >> > + rsdp_addr = (acpi_physical_address)table; >> > + else if (!(efi_guidcmp(guid, ACPI_20_TABLE_GUID))) >> > + return (acpi_physical_address)table; >> >> 'return' is not a function. >
Hi Ingo, I will remove the typecast in next version. But when doing more tests, I found a special condition. >Disregard this - I got confused by the type cast. > >The type cast is ugly nevertheless. 'table' is an 'unsigned long'. > >So 'acpi_physical_address' is basically an u64/u32, depending on >ACPI_MACHINE_WIDTH, right? > >Since this is x86, can ACPI_MACHINE_WIDTH ever get out of sync with the >native 'unsigned long' size? Not always. Here is define of acpi_physical_address: #if ACPI_MACHINE_WIDTH == 64 typedef u64 acpi_physical_address; #elif ACPI_MACHINE_WIDTH == 32 #ifdef ACPI_32BIT_PHYSICAL_ADDRESS typedef u32 acpi_physical_address; #else /* ACPI_32BIT_PHYSICAL_ADDRESS */ /* * It is reported that, after some calculations, the physical addresses can * wrap over the 32-bit boundary on 32-bit PAE environment. * https://bugzilla.kernel.org/show_bug.cgi?id=87971 */ typedef u64 acpi_physical_address; #endif That means in 32-bit with PAE, acpi_physical_address is u64. I set up a debian9-32bit with PAE in QEMU and found size of acpi_physical_address is 8. So they are not always sync. Of course, that doesn't influence the conclusion, since return table directly can always work. It will return u32 to u32, u64 to u64, or u32 to u64. So the typecast about acpi_physical_address can be removed. Thanks, Chao Fan > >If not then why not make the return type 'unsigned long', instead of >'acpi_physical_address' that you have to wade through a couple of headers >to figure out its true size. Does that cause complications elsewhere? > >I.e. the excessive type casts are ugly and make the code somewhat >fragile. > >Thanks, > > Ingo > >