From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932628AbcCHP7A (ORCPT ); Tue, 8 Mar 2016 10:59:00 -0500 Received: from vi37-28-154-113.vibiznes.pl ([37.28.154.113]:48842 "EHLO vps-vb.mhejs.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932275AbcCHP6u (ORCPT ); Tue, 8 Mar 2016 10:58:50 -0500 Message-ID: <56DEF6B0.9050902@maciej.szmigiero.name> Date: Tue, 08 Mar 2016 16:58:40 +0100 From: "Maciej S. Szmigiero" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.0 MIME-Version: 1.0 To: Darren Hart CC: platform-driver-x86@vger.kernel.org, linux-kernel , Kirill Tkhai Subject: Re: [PATCH 1/2] hp-wmi: fix unregister order in hp_wmi_rfkill_setup() once again References: <56DCB16C.80608@maciej.szmigiero.name> <20160308123649.GC31742@dvhart-mobl5.amr.corp.intel.com> In-Reply-To: <20160308123649.GC31742@dvhart-mobl5.amr.corp.intel.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Darren, Thanks for review, see also my comments below. On 08.03.2016 13:39, Darren Hart wrote: > On Sun, Mar 06, 2016 at 11:38:36PM +0100, Maciej S. Szmigiero wrote: >> --- a/drivers/platform/x86/hp-wmi.c >> +++ b/drivers/platform/x86/hp-wmi.c >> @@ -746,7 +746,7 @@ static int __init hp_wmi_rfkill_setup(struct platform_device *device) >> (void *) HPWMI_BLUETOOTH); >> if (!bluetooth_rfkill) { >> err = -ENOMEM; >> - goto register_wifi_error; >> + goto register_bluetooth_error; > > In this and all cases below, the goto label should match the situation, jumping > to register_bluetooth_error would be incorrect as we experienced a wifi error. Here we experienced an BT error - BT rkill allocation failed, so jump is to "register_bluetooth_error". The second jump to "register_bluetooth_error" is in another case of BT error: when its rfkill registration failed. It is the same label since if BT rfkill allocation had failed rfkill_destroy(bluetooth_rfkill) call in cleanup does nothing but we still need to possibly unregister WiFi rfkill that might have been registered in a previews block (and then fall through to next label to destroy WiFi rfkill). It would be possible to have separate jump labels skipping unnecessary rfkill_destroy() calls on allocation failure, but this would mean that we would have 7 labels in such small block of cleanup code. > A better solution would be to reorder the labels in the exit block > such that they enforce the necessary reverse order. Cleanup labels already are in reverse order with regard to registration: Registration: 1) WiFi, 2) BT, 3) WWAN, 5) GPS. Cleanup: 1) GPS, 2) WWAN, 3) BT, 4) WiFi. Best regards, Maciej Szmigiero