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

Groups > linux.kernel > #1341677

Re: [PATCH] regulator: core: fix error path of regulator_ena_gpio_free

Show all headers | View raw

On 24/02/16 08:26, Krzysztof Adamski wrote:
> On Tue, Feb 23, 2016 at 03:18:59PM +0000, Jon Hunter wrote:
>>
>> On 23/02/16 14:47, Krzysztof Adamski wrote:
>>> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@tieto.com>
>>> Reported-by: Jon Hunter <jonathanh@nvidia.com>
>>
>> Nit ... I think that order of the above should be reversed.
>>
> 
> Couldn't find any reference stating proper order of those tags and
> briefly looking at other commit messages shows this order as quite common.
> 
>>> ---
>>>  drivers/regulator/core.c | 8 +++-----
>>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
>>> index 6ee9ba4..d1e7859 100644
>>> --- a/drivers/regulator/core.c
>>> +++ b/drivers/regulator/core.c
>>> @@ -3919,7 +3919,7 @@ regulator_register(const struct regulator_desc
>>> *regulator_desc,
>>>          if (ret != 0) {
>>>              rdev_err(rdev, "Failed to request enable GPIO%d: %d\n",
>>>                   config->ena_gpio, ret);
>>> -            goto wash;
>>> +            goto clean;
>>>          }
>>>      }
>>>
>>> @@ -3942,7 +3942,7 @@ regulator_register(const struct regulator_desc
>>> *regulator_desc,
>>>
>>>      ret = set_machine_constraints(rdev, constraints);
>>>      if (ret < 0)
>>> -        goto scrub;
>>> +        goto wash;
>>>
>>>      if (init_data && init_data->supply_regulator)
>>>          rdev->supply_name = init_data->supply_regulator;
>>> @@ -3972,10 +3972,8 @@ out:
>>>  unset_supplies:
>>>      unset_regulator_supplies(rdev);
>>>
>>> -scrub:
>>> -    regulator_ena_gpio_free(rdev);
>>> -
>>>  wash:
>>> +    regulator_ena_gpio_free(rdev);
>>>      device_unregister(&rdev->dev);
>>>      /* device core frees rdev */
>>>      rdev = ERR_PTR(ret);
>>
>> What about the case where device_register() fails? I think you still
>> call clean and so you will leak the gpio?
>>
>> Jon
>>
> True. I couldn't find anything more clever than calling
> regulator_ena_gpio_free() in two paths like in an upcomming v2. Putting
> it inside of regulator_dev_release() won't entirely fix the problem
> either as this won't be called in this particular case
> (device_register() fail). I personally still prefer calling
> regulator_ena_gpio_free() inside of regulator_register insted of
> deffering it to regulator_dev_release() as it seems to be clearer to me.

Yes if you were to put regulator_ena_gpio_free() inside the
regulator_dev_release(), you would still need to call
regulator_ena_gpio_free() if the device_register fails. I see the way
you have it now there are two places you call regulator_ena_gpio_free()
in the error path. It is not a big deal, really either way.

Jon

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

Thread

Re: regulator: core: Request GPIO before creating sysfs entries Jon Hunter <jonathanh@nvidia.com> - 2016-02-23 15:20 +0100
  Re: regulator: core: Request GPIO before creating sysfs entries Krzysztof Adamski <krzysztof.adamski@tieto.com> - 2016-02-23 15:40 +0100
  [PATCH] regulator: core: fix error path of regulator_ena_gpio_free Krzysztof Adamski <krzysztof.adamski@tieto.com> - 2016-02-23 15:50 +0100
    Re: [PATCH] regulator: core: fix error path of  regulator_ena_gpio_free Jon Hunter <jonathanh@nvidia.com> - 2016-02-23 16:20 +0100
      Re: [PATCH] regulator: core: fix error path of  regulator_ena_gpio_free Krzysztof Adamski <krzysztof.adamski@tieto.com> - 2016-02-24 09:30 +0100
        Re: [PATCH] regulator: core: fix error path of  regulator_ena_gpio_free Jon Hunter <jonathanh@nvidia.com> - 2016-02-24 10:10 +0100
      [PATCH] regulator: core: fix error path of regulator_ena_gpio_free Krzysztof Adamski <krzysztof.adamski@tieto.com> - 2016-02-24 09:30 +0100
        Re: [PATCH] regulator: core: fix error path of  regulator_ena_gpio_free Jon Hunter <jonathanh@nvidia.com> - 2016-02-24 10:20 +0100
          [PATCH v3] regulator: core: fix crash in error path of regulator_register Krzysztof Adamski <krzysztof.adamski@tieto.com> - 2016-02-24 12:00 +0100
        Re: [PATCH] regulator: core: fix error path of  regulator_ena_gpio_free Jon Hunter <jonathanh@nvidia.com> - 2016-02-24 10:30 +0100
          Re: [PATCH] regulator: core: fix error path of  regulator_ena_gpio_free Mark Brown <broonie@kernel.org> - 2016-02-25 02:50 +0100

csiph-web