Re: [PATCH ?] ACPI: pr->id is unsigned

From: Thomas Renninger
Date: Tue Sep 16 2008 - 04:45:48 EST


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?
>
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/