[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v1 00/59] trivial unneeded labels cleanup
From: |
Max Reitz |
Subject: |
Re: [PATCH v1 00/59] trivial unneeded labels cleanup |
Date: |
Tue, 7 Jan 2020 10:43:09 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 |
On 06.01.20 19:23, 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.
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.
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
signature.asc
Description: OpenPGP digital signature
- [PATCH v1 40/59] hsb/hcd-ehci.c: remove unneeded labels, (continued)
- [PATCH v1 40/59] hsb/hcd-ehci.c: remove unneeded labels, Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 43/59] i386/amd_iommu.c: remove unneeded label in amdvi_int_remap_msi(), Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 50/59] rdma/rdma_backend.c: remove unneeded label in rdma_backend_init(), Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 16/59] block/gluster.c: remove unneeded 'exit' label, Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 21/59] block/backup.c remove unneeded 'out' label in backup_run(), Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 22/59] block/vmdk.c: remove unneeded labels, Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 28/59] qga/commands-win32.c: remove 'out' label in get_pci_info, Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 30/59] util/module.c: remove unneeded label in module_load_file(), Daniel Henrique Barboza, 2020/01/06
- Re: [PATCH v1 00/59] trivial unneeded labels cleanup,
Max Reitz <=
- Re: [PATCH v1 00/59] trivial unneeded labels cleanup, Eric Blake, 2020/01/10