Re: [PATCH 2/2] drivers: Simplify the return code

[Antti Palosaari <crope@iki.fi>]

On 05/19/2015 03:17 PM, Michael Büsch wrote:
> On Tue, 19 May 2015 08:05:56 -0400 (EDT)
> Federico Simoncelli <fsimonce@redhat.com> wrote:
>>> diff --git a/drivers/media/dvb-frontends/lgs8gxx.c
>>> b/drivers/media/dvb-frontends/lgs8gxx.c
>>> index 3c92f36ea5c7..9b0166cdc7c2 100644
>>> --- a/drivers/media/dvb-frontends/lgs8gxx.c
>>> +++ b/drivers/media/dvb-frontends/lgs8gxx.c
>>> @@ -544,11 +544,7 @@ static int lgs8gxx_set_mpeg_mode(struct lgs8gxx_state
>>> *priv,
>>>   	t |= clk_pol ? TS_CLK_INVERTED : TS_CLK_NORMAL;
>>>   	t |= clk_gated ? TS_CLK_GATED : TS_CLK_FREERUN;
>>>
>>> -	ret = lgs8gxx_write_reg(priv, reg_addr, t);
>>> -	if (ret != 0)
>>> -		return ret;
>>> -
>>> -	return 0;
>>> +	return lgs8gxx_write_reg(priv, reg_addr, t);
>>>   }
>>
>> Personally I prefer the current style because it's more consistent with all
>> the other calls in the same function (return ret when ret != 0).
>>
>> It also allows you to easily add/remove calls without having to deal with
>> the last special case return my_last_fun_call(...).
>>
>> Anyway it's not a big deal, I think it's your call.
>
>
> I agree. I also prefer the current style for these reasons. The compiler will also generate the same code in both cases.
> I don't think it really simplifies the code.
> But if you really insist on doing this change, go for it. You get my ack for fc0011


I am also against that kind of simplifications. Even it reduces line or 
two, it makes code more inconsistent, which means you have to make extra 
thinking when reading that code. I prefer similar repeating patterns as 
much as possible.

This is how I do it usually, even there is that extra last goto.

	ret = write_reg();
	if (ret)
		goto err;

	ret = write_reg();
	if (ret)
		goto err;
err:
	return ret;
};

regards
Antti

-- 
http://palosaari.fi/