* Re: [PATCH ?] ACPI: pr->id is unsigned
2008-09-16 1:32 [PATCH ?] ACPI: pr->id is unsigned roel kluin
@ 2008-09-15 20:26 ` Valdis.Kletnieks
2008-09-16 8:45 ` Thomas Renninger
0 siblings, 1 reply; 3+ messages in thread
From: Valdis.Kletnieks @ 2008-09-15 20:26 UTC (permalink / raw
To: roel kluin; +Cc: ak, lenb, linux-acpi, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 738 bytes --]
On Mon, 15 Sep 2008 21:32:20 EDT, roel kluin said:
> since pr->id is unsigned, shouldn't something like
> the patch below be applied?
> + BUG_ON((pr->id >= nr_cpu_ids) || ((unsigned long)pr->id < 0));
Under what conditions will the clause "(unsigned long)pr->id < 0)" be true,
and when will it be false? What will any sane optimizing compiler do?
And *sometimes*, the *real* bug is that pr->id should be a signed quantity,
not an unsigned one, and the cast is just papering over the issue.
In other words, the original line is almost certainly buggy. However, this
isn't the right fix. Somebody who actually understands the code will have to
decide what *should* be happening here (that's beyond my understanding of that
code)...
[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH ?] ACPI: pr->id is unsigned
@ 2008-09-16 1:32 roel kluin
2008-09-15 20:26 ` Valdis.Kletnieks
0 siblings, 1 reply; 3+ messages in thread
From: roel kluin @ 2008-09-16 1:32 UTC (permalink / raw
To: ak, lenb, linux-acpi, linux-kernel
since pr->id is unsigned, shouldn't something like
the patch below be applied?
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index e36422a..75c0f76 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -667,7 +667,7 @@ static int __cpuinit acpi_processor_start(struct acpi_device *device)
return 0;
}
- BUG_ON((pr->id >= nr_cpu_ids) || (pr->id < 0));
+ BUG_ON((pr->id >= nr_cpu_ids) || ((unsigned long)pr->id < 0));
/*
* Buggy BIOS check
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH ?] ACPI: pr->id is unsigned
2008-09-15 20:26 ` Valdis.Kletnieks
@ 2008-09-16 8:45 ` Thomas Renninger
0 siblings, 0 replies; 3+ messages in thread
From: Thomas Renninger @ 2008-09-16 8:45 UTC (permalink / raw
To: Valdis.Kletnieks; +Cc: roel kluin, ak, lenb, linux-acpi, linux-kernel
On Monday 15 September 2008 22:26:45 Valdis.Kletnieks@vt.edu wrote:
> On Mon, 15 Sep 2008 21:32:20 EDT, roel kluin said:
> > since pr->id is unsigned, shouldn't something like
> > the patch below be applied?
> >
> > + BUG_ON((pr->id >= nr_cpu_ids) || ((unsigned long)pr->id < 0));
>
> Under what conditions will the clause "(unsigned long)pr->id < 0)" be true,
> and when will it be false? What will any sane optimizing compiler do?
>
> And *sometimes*, the *real* bug is that pr->id should be a signed quantity,
> not an unsigned one, and the cast is just papering over the issue.
>
> In other words, the original line is almost certainly buggy. However, this
> isn't the right fix. Somebody who actually understands the code will have
> to decide what *should* be happening here (that's beyond my understanding
> of that code)...
Just removing "pr->id < 0" condition should be the right fix.
For such an easy patch in the ACPI subsystem it's enough to only post to
the linux-acpi mailing list, no need to bother the whole world.
Thanks for finding this, do you mind to repost a new version?
Thomas
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-09-16 8:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-16 1:32 [PATCH ?] ACPI: pr->id is unsigned roel kluin
2008-09-15 20:26 ` Valdis.Kletnieks
2008-09-16 8:45 ` Thomas Renninger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).