[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
ado.
> 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.
Maybe.
Re: [PATCH v1 00/59] trivial unneeded labels cleanup,
Markus Armbruster <=
[PATCH v1 06/59] mips-semi.c: remove 'uhi_done' label in helper_do_semihosting(), Daniel Henrique Barboza, 2020/01/06
[PATCH v1 17/59] block/dmg.c: remove unneeded 'fail' label in dmg_read_mish_block(), Daniel Henrique Barboza, 2020/01/06
[PATCH v1 19/59] block/ssh.c: remove unneeded labels, Daniel Henrique Barboza, 2020/01/06
[PATCH v1 03/59] kvm-all.c: remove unneeded labels, Daniel Henrique Barboza, 2020/01/06
[PATCH v1 01/59] spapr.c: remove 'out' label in spapr_dt_cas_updates(), Daniel Henrique Barboza, 2020/01/06