On 21/05/2015 08:09, Ingo Molnar wrote: > > * Nicholas Krause <xerofoify@xxxxxxxxx> wrote: > >> In order to make code paths easier to read in the function, >> kvm_create_lapic we return -ENOMEM when unable to allocate >> memory for a kvm_lapic structure pointer directly. This >> makes the code easier to read and cleaner then jumping >> to a goto label at the end of the function's body for >> returning just the error code, -ENOMEM. >> >> Signed-off-by: Nicholas Krause <xerofoify@xxxxxxxxx> >> --- >> arch/x86/kvm/lapic.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c >> index 629af0f..88d0cce 100644 >> --- a/arch/x86/kvm/lapic.c >> +++ b/arch/x86/kvm/lapic.c >> @@ -1687,7 +1687,7 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu) >> >> apic = kzalloc(sizeof(*apic), GFP_KERNEL); >> if (!apic) >> - goto nomem; >> + return -ENOMEM; >> >> vcpu->arch.apic = apic; >> >> @@ -1718,7 +1718,6 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu) >> return 0; >> nomem_free_apic: >> kfree(apic); >> -nomem: >> return -ENOMEM; >> } > > NAK! > > You just half destroyed the nice error handling cascade of labels. Right. What could be done, is always going through kfree(apic), because it is okay to free NULL. So the nomem label moves up, and the nomem_free_apic label is not necessary anymore. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
