[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 00/59] trivial unneeded labels cleanup

From: Markus Armbruster
Subject: Re: [PATCH v1 00/59] trivial unneeded labels cleanup
Date: Mon, 13 Jan 2020 16:26:30 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux)

Corey Minyard <address@hidden> writes:

> On Mon, Jan 06, 2020 at 03:23:26PM -0300, Daniel Henrique Barboza wrote:
>> Hello,
>> This is the type of cleanup I've contributed to Libvirt
>> during the last year. Figured QEMU also deserves the same
>> care.
>> The idea here is remove unneeded labels. By 'unneeded' I
>> mean labels that does nothing but a 'return' call. One
>> common case is something like this:
>> if ()
>>     goto cleanup;
>> [...]
>>  cleanup:
>>     return 0;
>> This code can be simplified to:
>> if ()
>>     return 0;
>> Which is cleaner and requires less brain cycles to wonder
>> whether the 'cleanup' label does anything special, such
>> as a heap memory cleanup.
> I would disagree with this analysis.  To me, I often wonder
> when I have to add cleanup code to a routine whether there is
> some hidden return in the middle of the function.  That's a lot
> harder to spot than just looking for the cleanup label at the
> end of the function to see what it does.

A cleanup label does not preclude existence of return.  You have to
check for return anyway.

>                                           For non-trivial
> functions I prefer to have one point of return at the end
> (and maybe some minor checks with returns right at the beginning).
> I'm not adamant about this, just my opinion.

I'm in Daniel's camp: if there's no need for cleanup, return without

> But if we are going to fix things like this, it might be best to add
> them to the coding style document and checkpatch script.  Otherwise
> these sorts of things will just continue.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]