On Sat, Feb 4, 2017 at 3:14 AM, Darren Hart <dvhart@xxxxxxxxxxxxx> wrote:
> On Thu, Jan 26, 2017 at 03:33:01PM +0800, Alex Hung wrote:
>> New firmwares include a feature called 5 button array that supports
>> super key, volume up/down, rotation lock and power button. Especially,
>> support for this feature is required to fix power button on some recent
>> systems.
>> +static int intel_button_array_input_setup(struct platform_device *device)
>> +{
>> + struct intel_hid_priv *priv = dev_get_drvdata(&device->dev);
>> + int ret;
>> +
>> + /* Setup input device for 5 button array */
>> + priv->array = input_allocate_device();
>> + if (!priv->array)
>> + return -ENOMEM;
>> +
>> + ret = sparse_keymap_setup(priv->array, intel_array_keymap, NULL);
>> + if (ret)
>> + goto err_free_array_device;
>> +
>> + priv->array->dev.parent = &device->dev;
>> + priv->array->name = "Intel HID 5 button array";
>> + priv->array->id.bustype = BUS_HOST;
>> +
>> + ret = input_register_device(priv->array);
>> + if (ret)
>> + goto err_free_array_device;
>> +
>> + return 0;
>> +
>> +err_free_array_device:
>> + input_free_device(priv->array);
>> + return ret;
>
> This return path is more complex than it could be, since you test for ret before
> return anyway:
>
> out:
> if (ret)
> input_free_device(priv->array);
> return ret;
>
> There is no need for a second return point that I can see. Same for the
> hid_input_setup function. We can remove 8 lines.
Darren, if I didn't miss anything the function is purely used in
->probe() path and thus devm_ version of input_allocate_device() would
make this error path gone.
--
With Best Regards,
Andy Shevchenko
