From: Linus Torvalds on


On Sat, 20 Mar 2010, Peter Zijlstra wrote:
>
> cpuhw = &per_cpu(cpu_hw_events, cpu);
> + if (!cpuhw)
> + return;

How can an address-of expression be NULL?

Yes, 'per_cpu()' is magic, but it shouldn't be possible to be _that_
magic. It's rather against the whole C model.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Linus Torvalds on


On Sat, 20 Mar 2010, Rafael J. Wysocki wrote:
>
> The appended patch fixes the breakage for me too.

Please don't use a 'goto' for something like this.

> raw_spin_lock(&amd_nb_lock);
>
> + if (!cpuhw->amd_nb)
> + goto unlock;
> +
> if (--cpuhw->amd_nb->refcnt == 0)
> kfree(cpuhw->amd_nb);
>
> cpuhw->amd_nb = NULL;
>
> + unlock:
> raw_spin_unlock(&amd_nb_lock);
> }

Just do

raw_spin_lock(&amd_nb_lock);
if (cpuhw->amd_nb) {
if (!--cpuhw->amd_nb->refcnt)
kfree(cpuhw->amd_nb);
cpuhw->amd_nb = NULL;
}
raw_spin_unlock(&amd_nb_lock);

instead. Much more readable.

Let's keep 'goto' for cases where we have error returns that we don't want
to nest, not trivial stuff like this.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/