Messages in this thread | | | Date | Tue, 18 Aug 2009 22:17:03 +0100 | Subject | Re: [PATCH 2/3] Add rfkill support to compal-laptop | From | Alan Jenkins <> |
| |
On 8/18/09, Mario Limonciello <mario_limonciello@dell.com> wrote: > Hi Alan & Marcel: > > Alan Jenkins wrote: >> Also, you're missing the calls to rfkill_destroy() here. >> >> Whew, I think that's everything. I hope you find the feedback useful, >> despite it being a little fragmented. >> >> > Thanks for all the feedback. I think i've addressed all of the concerns > that were pointed out. I appreciate the pointer to scripts/cleanpatch, > that does significantly help in finding whitespace problems that the > naked eye just browses over.
I think you've addressed most of the comments on your first patch. There are still problems with the error handling though.
+static int setup_rfkill(void) +{ + int ret; + + wifi_rfkill = rfkill_alloc("compal-wifi", &compal_device->dev, + RFKILL_TYPE_WLAN, &compal_rfkill_ops, + (void *) WLAN_MASK); + if (!wifi_rfkill) { + ret = -ENOMEM; + goto err_wifi; + } + ret = rfkill_register(wifi_rfkill); + if (ret) { + rfkill_unregister(wifi_rfkill); + goto err_wifi; + }
If you fail to register an rfkill device, you don't need to unregister that rfkill device...
+ + bt_rfkill = rfkill_alloc("compal-bluetooth", &compal_device->dev, + RFKILL_TYPE_BLUETOOTH, &compal_rfkill_ops, + (void *) BT_MASK); + if (!bt_rfkill) { + ret = -ENOMEM; + goto err_bt; + } + ret = rfkill_register(bt_rfkill); + if (ret) { + rfkill_unregister(bt_rfkill); + goto err_bt; + }
and the same applies here
+ + return 0; + +err_bt: + rfkill_destroy(bt_rfkill); +
... but you *do* need to unregister wifi_rfkill here, before you go on to destroy it.
+err_wifi: + rfkill_destroy(wifi_rfkill); + + return ret; +}
Regards Alan
| |