On Monday, August 16, 2021 11:05:00 AM CEST Dan Carpenter wrote:
> To be honest, I prefered the original.
>
> foo = alloc();
> if (!foo) {
> ret = -EWHATEVER;
> goto free_last_thing;
> }
>
> I like this style of error handling because all the information is
> there. You don't need to scroll down.
>
Thinking deeper about this style of error handling, I find that Dan is quite
right in preferring readability over removal of (technically unnecessary)
temporary variables. Perhaps the trade-off between brevity and readability
should (in general) favor the latter.
Furthermore, those temporary variables make the code easily adaptable/
extensible in cases where, in future revisions of the code, there will be more
different errors to handle.
However, it's mainly a matter of style, so...
>
> I don't really care about this specific patch at all. It's a small
> thing. But we had someone come through who was sort of obsessed with
> removing these sorts of variables. Just because you can remove a
> variable doesn't necessarily make the code more readable.
>
> If you're doing the work and maintaining the driver you get to choose
> your own style to some extent.
>
I agree: choose your own style (to some extent).
Thanks,
Fabio
>
> But I don't want to encourage people to
> start sending these sort of patches more generally.
>
> regards,
> dan carpenter
