[PATCH 09/50] staging: omap-thermal: make a omap_bandgap_power with only one exit point

[Dan Carpenter]

On Sat, Mar 16, 2013 at 08:39:11AM -0400, Eduardo Valentin wrote:
> Hey Dan,
> 
> On 15-03-2013 17:22, Dan Carpenter wrote:
> >On Fri, Mar 15, 2013 at 08:59:57AM -0400, Eduardo Valentin wrote:
> >>Change the way the omap_bandgap_power is written so that it has only
> >>one exit entry (Documentation/CodingStyle).
> >>
> >
> >It's only if there is an unlock or something that you should do
> >this.  Otherwise the pointless bunny hop is misleading and annoying.
> 
> Well, if that is the case the Chapter 7 needs to be rewritten, don't
> you think? The way it is stated, it is clear that it is a design
> decision to use it for keeping only one exit point (quoting):
> 
> "Albeit deprecated by some people, the equivalent of the goto statement is
> used frequently by compilers in form of the unconditional jump instruction.
> 
> The goto statement comes in handy when a function exits from multiple
> locations and some common work such as cleanup has to be done.
> 
> The rationale is:
> 
> - unconditional statements are easier to understand and follow
> - nesting is reduced
> - errors by not updating individual exit points when making
>     modifications are prevented
> - saves the compiler work to optimize redundant code away ;)"
> 
> I believe this patch falls into at least three of the above rationale.

There isn't any cleanup work done.  That's what I was explaining
about.  When I see a goto I expect cleanup work to be done, but in
this case it was misleading.

Where you would do the one exit point is when there is cleanup:

	ret = function_one();
	if (ret)
		goto unlock;

	ret = function_two();
	if (ret)
		goto unlock;


That's better than:
	ret = function_one();
	if (ret) {
		mutex_unlock();
		return ret;
	}

	ret = function_two();
	if (ret) {
		mutex_unlock();
		return ret;
	}

On the one hand, I think the documentation should be updated because
obviously people get confused by it.  But on the other hand, to me
the documentation is already pretty clear.  There is no style
guideline that says every function should only have a single return.
That would be silly.

regards,
dan carpenter