Re: [PATCH 2/3] Add rfkill support to compal-laptop

From: Johannes Berg <johannes@sipsolutions.net>
To: Mario Limonciello <mario_limonciello@dell.com>
Cc: Alan Jenkins <sourcejedi.lkml@googlemail.com>,
	Marcel Holtmann <marcel@holtmann.org>,
	cezary.jackiewicz@gmail.com, linux-acpi@vger.kernel.org,
	linux-kernel <linux-kernel@vger.kernel.org>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH 2/3] Add rfkill support to compal-laptop
Date: Wed, 19 Aug 2009 19:13:32 +0200	[thread overview]
Message-ID: <1250702012.8073.1.camel@johannes.local> (raw)
In-Reply-To: <4A8C2C6C.2020007@dell.com>

[-- Attachment #1: Type: text/plain, Size: 685 bytes --]

Hi Mario,

First, let me say I agree with Alan, the option 1 is more desirable if
possible to do with the hardware. But this looks ok from an rfkill POV
now, except there's a small bug:

> +       ret = setup_rfkill();
> +       if (ret)
> +               printk(KERN_WARNING "compal-laptop: Unable to setup
> rfkill\n");

That doesn't error out, so

> +       rfkill_unregister(wifi_rfkill);
> +       rfkill_destroy(wifi_rfkill);
> +       rfkill_unregister(bt_rfkill);
> +       rfkill_destroy(bt_rfkill);

this will crash without NULL checks.

(and you have to explicitly assign NULL in setup_rfkill() too, when
bluetooth fails and wifi is freed)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]