Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1353165

Re: [PATCH 1/2] hp-wmi: fix unregister order in hp_wmi_rfkill_setup() once again

Show all headers | View raw

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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 1/2] hp-wmi: fix unregister order in hp_wmi_rfkill_setup()  once again "Maciej S. Szmigiero" <mail@maciej.szmigiero.name> - 2016-03-06 23:40 +0100
  Re: [PATCH 1/2] hp-wmi: fix unregister order in  hp_wmi_rfkill_setup() once again Darren Hart <dvhart@infradead.org> - 2016-03-08 13:50 +0100
    Re: [PATCH 1/2] hp-wmi: fix unregister order in hp_wmi_rfkill_setup()  once again "Maciej S. Szmigiero" <mail@maciej.szmigiero.name> - 2016-03-08 17:00 +0100
      Re: [PATCH 1/2] hp-wmi: fix unregister order in  hp_wmi_rfkill_setup() once again Darren Hart <dvhart@infradead.org> - 2016-03-08 18:10 +0100

csiph-web