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

[Thomas Renninger <trenn@suse.de>]

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