Re: [PATCH 1/3] ACPI / processor: remove incorrect comparison of phys_id

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Hanjun,

On 26/11/14 09:53, Hanjun Guo wrote:
Hi Sudeep,

On 2014-11-25 22:48, Sudeep Holla wrote:
phys_id in acpi_processor structure is unsigned 32-bit integer,
comparing it with signed value is incorrect.

Yes, this is a bug :)

But the phys_id in acpi_processor structure should be signed value
because acpi_get_phys_id() will return -1 if there if no CPU entry
found in MADT table.

I found the id in acpi_processor structure should also be signed
value by unsigned now, we should fix that too.


I tend to disagree, since the physical(h/w) and logical identifiers are
unsigned integers, the structure elements must also be the same. Though
for checking the correctness of those identifiers, the functions can
manage through signed values or any other alternates IMO(i.e. more
implementation details)

Let's see what's Rafael's opinion on this.


This patch removes that incorrect comparision in
acpi_processor_hotadd_init as the lone user of this function is
already checking for correctness of phys_id before calling it.

if (apic_id < 0) acpi_handle_debug(pr->handle, "failed to get CPU
APIC ID.\n");

it only check the value and print debug message but no returns, so I
think the check in the following patch is still needed.


Agreed, but that's something we need to fix in the logic and not by
changing these identifiers to signed values in the structures.

Regards,
Sudeep



--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux