Re: [PATCH] staging: r8188eu: Remove unnecessary ret variable in rtw_drv_init()

[Fabio M. De Francesco]

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