|
From: | Daniel Henrique Barboza |
Subject: | Re: [PATCH v1 00/59] trivial unneeded labels cleanup |
Date: | Thu, 9 Jan 2020 17:16:32 -0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 |
On 1/7/20 6:43 AM, Max Reitz wrote:
On 06.01.20 19:23, Daniel Henrique Barboza wrote:
[...]
For me, it doesn’t require any brain cycles, because I generally just assume the cleanup label will do the right thing. OTOH, a return statement may make me invest some some brain cycles, because maybe there is something to be cleaned up and it isn’t done here.Another common case uses a variable to set a return value, generally an error, then return: if () { ret = -ENOENT; goto out; } [..] out: return ret; Likewise, it is clearer to just 'return -ENOENT' instead of jumping to a label. There are other cases being handled in these patches, but these are the most common.I find it clearer from the perspective of “less LoC”, but I find it less clear from the perspective of “Is this the right way to clean up?”. Even on patch 15 (which you say isn’t too much of a debate), I don’t find the change to make things any clearer. Just less verbose.
Fair enough. As I said in the cover, all this patches makes no functional changes, just a clean up aiming for less LoC (and more clarity, at least in my opinion). I am aware all the good sides of keeping the code as is, such as being easier to debug (although I would argue that an explicit trace_event call is better than keeping verbose code 'just in case'), or goto usage to keep just one return statement per function. I am also aware that the existing QEMU code base has a mesh of styles. What I'm proposing here isn't a 'my way is better' case by any means, but it's not something unprecedented in the existing code base either. Since there's no QEMU code guideline imposing that a function should have only one 'return' statement regardless of how many 'goto' calls are needed, or a guideline that discourages 'goto' calls regardless of how many 'return' calls are needed, in the end it's a matter of seeing what fits the function/code better. In the maintainers opinion, of course. Thanks, DHB
I suppose none of this would matter if we used __attribute__((cleanup)) everywhere and simply never had to clean up anything manually. But as long as we don’t and require cleanup paths in many places, I disagree that they require more brain cycles than plain return statements. Max
[Prev in Thread] | Current Thread | [Next in Thread] |