Thomas Renninger wrote: > On Monday 15 September 2008 22:26:45 Valdis.Kletnieks@xxxxxx 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? ah, yes, remove it, I wasn't thinking. :-/ >> 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)... Thanks for correcting me, I should have verified that. A prior mail, http://lkml.org/lkml/2008/9/9/136 brought me on a wrong line of thought: [Quote (with ret as an unsigned long)] if ((unsigned long)ret >= -ERESTART_RESTARTBLOCK) This and similar ones are not wrong. The -constant is converted to unsigned (which is a well-defined operation) after which an >=u (greater-or-equal unsigned) is performed. [End Quote] > 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. Ok, removed the rest of the world :-) > Thanks for finding this, do you mind to repost a new version? > > Thomas > pr->id is unsigned Signed-off-by: Roel Kluin <roel.kluin@xxxxxxxxx> --- diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index ee68ac5..9f203d3 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); /* * Buggy BIOS check -- 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
