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

Groups > linux.kernel > #1625410 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH] regulator/core.c: remove the else statement hubiaoyong <hubiaoyong@gmail.com> - 2017-04-18 17:50 +0200
    Re: [PATCH] regulator/core.c: remove the else statement Mark Brown <broonie@kernel.org> - 2017-04-18 18:00 +0200
      Re: [PATCH] regulator/core.c: remove the else statement Joe Perches <joe@perches.com> - 2017-04-18 18:30 +0200
        Re: [PATCH] regulator/core.c: remove the else statement Mark Brown <broonie@kernel.org> - 2017-04-19 18:30 +0200

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

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

[toc] | [next] | [standalone]

#1625421

[Multipart message — attachments visible in raw view] — view raw

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?

[toc] | [prev] | [next] | [standalone]

#1625432

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--;
	}

[toc] | [prev] | [next] | [standalone]

#1626474

[Multipart message — attachments visible in raw view] — view raw

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web