From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754420AbbEUMWa (ORCPT ); Thu, 21 May 2015 08:22:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50302 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753001AbbEUMW1 (ORCPT ); Thu, 21 May 2015 08:22:27 -0400 Message-ID: <555DCDF4.3000005@redhat.com> Date: Thu, 21 May 2015 14:22:12 +0200 From: Paolo Bonzini User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Ingo Molnar , Nicholas Krause CC: gleb@kernel.org, x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] kvm:Return -ENOMEM directly for the function, kvm_create_lapic References: <1432169486-31846-1-git-send-email-xerofoify@gmail.com> <20150521060940.GB5615@gmail.com> In-Reply-To: <20150521060940.GB5615@gmail.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 21/05/2015 08:09, Ingo Molnar wrote: > > * Nicholas Krause 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 >> --- >> 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