Re: [PATCH v2 35/44] error: Eliminate error_propagate() with Coccinelle, part 2
Eric Blake writes: > On 7/2/20 10:49 AM, Markus Armbruster wrote: >> When all we do with an Error we receive into a local variable is >> propagating to somewhere else, we can just as well receive it there >> right away. The previous commit did that with a Coccinelle script I >> consider fairly trustworthy. This commit uses the same script with >> the matching of return taken out, i.e. we convert >> >> if (!foo(..., &err)) { >> ... >> error_propagate(errp, err); >> ... >> } >> >> to >> >> if (!foo(..., errp)) { >> ... >> ... >> } >> >> This is unsound: @err could still be read between afterwards. I don't >> know how to express "no read of @err without an intervening write" in >> Coccinelle. Instead, I manually double-checked for uses of @err. >> >> Suboptimal line breaks tweaked manually. qdev_realize() simplified >> further to placate scripts/checkpatch.pl. >> >> Signed-off-by: Markus Armbruster >> --- >> block.c | 6 ++ >> block/blkdebug.c | 7 ++- >> block/blklogwrites.c | 3 +-- >> block/blkverify.c| 3 +-- >> block/crypto.c | 4 +--- >> block/file-posix.c | 6 ++ >> block/file-win32.c | 6 ++ >> block/gluster.c | 4 +--- >> block/iscsi.c| 3 +-- >> block/nbd.c | 8 ++-- >> block/qcow2.c| 13 - >> block/raw-format.c | 4 +--- >> block/sheepdog.c | 8 ++-- >> block/ssh.c | 4 +--- >> block/throttle.c | 4 +--- >> block/vmdk.c | 4 +--- >> block/vpc.c | 3 +-- >> block/vvfat.c| 3 +-- >> blockdev.c | 3 +-- >> hw/intc/xics.c | 4 +--- >> hw/vfio/pci.c| 3 +-- >> net/tap.c| 3 +-- >> qom/object.c | 4 +--- >> 23 files changed, 32 insertions(+), 78 deletions(-) > > Small enough to review each instance. > > Reviewed-by: Eric Blake I tried *really* hard to make part 1's script powerful and safe, to give the unsafe / manual parts (this commit and next) a chance of meaningful review. Thanks for providing it!
Re: [PATCH v2 35/44] error: Eliminate error_propagate() with Coccinelle, part 2
On 7/2/20 10:49 AM, Markus Armbruster wrote: When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. The previous commit did that with a Coccinelle script I consider fairly trustworthy. This commit uses the same script with the matching of return taken out, i.e. we convert if (!foo(..., &err)) { ... error_propagate(errp, err); ... } to if (!foo(..., errp)) { ... ... } This is unsound: @err could still be read between afterwards. I don't know how to express "no read of @err without an intervening write" in Coccinelle. Instead, I manually double-checked for uses of @err. Suboptimal line breaks tweaked manually. qdev_realize() simplified further to placate scripts/checkpatch.pl. Signed-off-by: Markus Armbruster --- block.c | 6 ++ block/blkdebug.c | 7 ++- block/blklogwrites.c | 3 +-- block/blkverify.c| 3 +-- block/crypto.c | 4 +--- block/file-posix.c | 6 ++ block/file-win32.c | 6 ++ block/gluster.c | 4 +--- block/iscsi.c| 3 +-- block/nbd.c | 8 ++-- block/qcow2.c| 13 - block/raw-format.c | 4 +--- block/sheepdog.c | 8 ++-- block/ssh.c | 4 +--- block/throttle.c | 4 +--- block/vmdk.c | 4 +--- block/vpc.c | 3 +-- block/vvfat.c| 3 +-- blockdev.c | 3 +-- hw/intc/xics.c | 4 +--- hw/vfio/pci.c| 3 +-- net/tap.c| 3 +-- qom/object.c | 4 +--- 23 files changed, 32 insertions(+), 78 deletions(-) Small enough to review each instance. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org