Prev: x86 / perf: Fix suspend to RAM on HP nx6325
Next: [PATCH 9/9] Staging: comedi: cb_pcidas64: fixed a coding style missed in the previosu patch
From: Linus Torvalds on 20 Mar 2010 14:00 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 21 Mar 2010 13:40
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/ |