On Sun, Jun 21, 2015 at 10:39:27AM +0200, Pali Rohár wrote:
> Make sure that return value of all SMBIOS calls are properly checked and
> do not continue of processing (received) information if call failed.
>
> Signed-off-by: Pali Rohár <pali.rohar@xxxxxxxxx>
> ---
> drivers/platform/x86/dell-laptop.c | 61 +++++++++++++++++++++++++++++++-----
> 1 file changed, 54 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
...
> @@ -677,11 +690,16 @@ static const struct file_operations dell_debugfs_fops = {
> static void dell_update_rfkill(struct work_struct *ignored)
> {
> int status;
> + int ret;
>
> get_buffer();
> dell_send_request(buffer, 17, 11);
> + ret = buffer->output[0];
> status = buffer->output[1];
Parallel to previous blocks, you can release_buffer() here...
>
> + if (ret != 0)
> + goto out;
And just return here
> +
> if (wifi_rfkill) {
> dell_rfkill_update_hw_state(wifi_rfkill, 1, status);
> dell_rfkill_update_sw_state(wifi_rfkill, 1, status);
> @@ -695,6 +713,7 @@ static void dell_update_rfkill(struct work_struct *ignored)
> dell_rfkill_update_sw_state(wwan_rfkill, 3, status);
> }
>
> + out:
And drop this label.
> release_buffer();
> }
> static DECLARE_DELAYED_WORK(dell_rfkill_work, dell_update_rfkill);
> @@ -755,13 +774,35 @@ static int __init dell_setup_rfkill(void)
> return 0;
>
> get_buffer();
> +
> dell_send_request(buffer, 17, 11);
> + ret = buffer->output[0];
> status = buffer->output[1];
> + if (ret != 0) {
> + /* dell wireless info smbios call is not working */
> + /* so there is no support for rfkill */
Please follow coding style for multi-line comments.
> + release_buffer();
> + return 0;
> + }
> +
> buffer->input[0] = 0x2;
> dell_send_request(buffer, 17, 11);
> + ret = buffer->output[0];
> hwswitch_state = buffer->output[1];
> +
> release_buffer();
>
> + if (ret != 0) {
Just "if (ret)" is more typical
> + /* dell wireless switch smbios call is not working */
> + if (force_rfkill) {
> + /* clear all hw-controlled bits */
> + hwswitch_state &= ~7;
> + } else {
> + /* rfkill is only tested on laptops with a hwswitch */
> + return 0;
> + }
Save an additional indent block and all the braces with:
/* rfkill is only tested on laptops with a hwswitch */
if (!force_rfkill)
return 0
/* clear all hw-controlled bits */
hwswitch_state &= ~7;
> + }
> +
> if (!(status & BIT(0))) {
> if (force_rfkill) {
> /* No hwsitch, clear all hw-controlled bits */
> @@ -931,6 +972,8 @@ static int dell_send_intensity(struct backlight_device *bd)
> else
> dell_send_request(buffer, 1, 1);
>
> + ret = dell_smi_error(buffer->output[0]);
> +
> out:
> release_buffer();
> return ret;
> @@ -953,7 +996,10 @@ static int dell_get_intensity(struct backlight_device *bd)
> else
> dell_send_request(buffer, 0, 1);
>
> - ret = buffer->output[1];
> + if (buffer->output[0])
> + ret = dell_smi_error(buffer->output[0]);
> + else
> + ret = buffer->output[1];
This is OK, but this block reverses the ret/status terms applied to output[0]
and output[1] which is a little confusing.
>
> out:
> release_buffer();
> @@ -2087,7 +2133,8 @@ static int __init dell_init(void)
> buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
> if (buffer->input[0] != -1) {
> dell_send_request(buffer, 0, 2);
> - max_intensity = buffer->output[3];
> + if (buffer->output[0] == 0)
> + max_intensity = buffer->output[3];
> }
> release_buffer();
>
> --
> 1.7.9.5
>
>
--
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
