Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To:     Nathan Chancellor <nathan@kernel.org>,
        Dan Carpenter <dan.carpenter@oracle.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Phillip Potter <phil@philpotter.co.uk>,
        Larry Finger <Larry.Finger@lwfinger.net>,
        linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: r8188eu: Remove unnecessary ret variable in rtw_drv_init()
Date:   Mon, 16 Aug 2021 12:15:56 +0200
Message-ID: <2121129.EM0CHCvl88@localhost.localdomain>
In-Reply-To: <20210816090500.GG1931@kadam>
References: <20210813201418.4018631-1-nathan@kernel.org> <20210816090500.GG1931@kadam>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

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