[PATCH] regulator/core.c: remove the else statement

[PATCH] regulator/core.c: remove the else statement

[hubiaoyong]

in the function regulator_ena_gpio_free, the if branch contains
the return statement, so remove the else statement.

Signed-off-by: hubiaoyong <hubiaoyong@gmail.com>
---
 drivers/regulator/core.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 53d4fc7..de3d07a 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2007,9 +2007,8 @@ static void regulator_ena_gpio_free(struct regulator_dev *rdev)
 				kfree(pin);
 				rdev->ena_pin = NULL;
 				return;
-			} else {
-				pin->request_count--;
 			}
+			pin->request_count--;
 		}
 	}
 }
-- 
1.7.1

Re: [PATCH] regulator/core.c: remove the else statement

[Mark Brown]

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

On Tue, Apr 18, 2017 at 11:39:34PM +0800, hubiaoyong wrote:
> in the function regulator_ena_gpio_free, the if branch contains
> the return statement, so remove the else statement.

Why is it a benefit to make this change?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] regulator/core.c: remove the else statement

[Joe Perches]

On Tue, 2017-04-18 at 16:49 +0100, Mark Brown wrote:
> On Tue, Apr 18, 2017 at 11:39:34PM +0800, hubiaoyong wrote:
> > in the function regulator_ena_gpio_free, the if branch contains
> > the return statement, so remove the else statement.
> 
> Why is it a benefit to make this change?

In general, reducing source code indentation is a good thing.

The logic today is:

	/* Free the GPIO only in case of no use */
	list_for_each_entry_safe(pin, n, &regulator_ena_gpio_list, list) {
		if (pin->gpiod == rdev->ena_pin->gpiod) {
			if (pin->request_count <= 1) {
				pin->request_count = 0;
				gpiod_put(pin->gpiod);
				list_del(&pin->list);
				kfree(pin);
				rdev->ena_pin = NULL;
				return;
			} else {
				pin->request_count--;
			}
		}
	}

Perhaps it's better written as:

	/* Free the GPIO only in case of no use */
	list_for_each_entry_safe(pin, n, &regulator_ena_gpio_list, list) {
		if (pin->gpiod != rdev->ena_pin->gpiod)
			continue;
		if (pin->request_count <= 1) {
			pin->request_count = 0;
			gpiod_put(pin->gpiod);
			list_del(&pin->list);
			kfree(pin);
			rdev->ena_pin = NULL;
			return;
		}
		pin->request_count--;
	}

Re: [PATCH] regulator/core.c: remove the else statement

[Mark Brown]

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

On Wed, Apr 19, 2017 at 08:53:05PM +0800, Biaoyong Hu wrote:

> kfree(pin); pin will be freed. So, "pin->request_count = 0;" should be
> removed?

Yes, it's redundant.

Please don't top post, reply in line with needed context.  This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]