linux-kernel - Re: [PATCH] staging: r8188eu: Remove unnecessary ret variable in rtw_drv

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 16 Aug 2021 12:15:56 +0200
From:   "Fabio M. De Francesco" <fmdefrancesco@...il.com>
To:     Nathan Chancellor <nathan@...nel.org>,
        Dan Carpenter <dan.carpenter@...cle.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Phillip Potter <phil@...lpotter.co.uk>,
        Larry Finger <Larry.Finger@...inger.net>,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: r8188eu: Remove unnecessary ret variable in rtw_drv_init()

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