Changes v1 -> v2: * The Coccinelle scripts were simplified by using "when" constraints to detect when a variable is not used elsewhere inside the function. * Added script to remove unnecessary variables for function return value. * Coccinelle scripts added to scripts/coccinelle.
Changes v2 -> v3 on patch 2/3: * Remove unused metavariable from script * Do changes only if errp is not touched before the error_setg() call (so we are sure *errp is not set and error_setg() won't abort) * Changes dropped from v2: * block.c:bdrv_create_co_entry() * block.c:bdrv_create_file() * blockdev.c:qmp_blockdev_mirror() Changes v2 -> v3 on patch 3/3: * Not a RFC anymore * Used --keep-comments option * Instead of using: - VAR = E; + return E; use: - VAR = + return E This makes Coccinelle keep the existing formatting on some cases. * Manual fixups Diff from v2 below: diff --git a/scripts/coccinelle/remove_local_err.cocci b/scripts/coccinelle/remove_local_err.cocci index 5f0b6c0..9261c99 100644 --- a/scripts/coccinelle/remove_local_err.cocci +++ b/scripts/coccinelle/remove_local_err.cocci @@ -2,18 +2,20 @@ // direct usage of errp argument @@ +identifier F; expression list ARGS; expression F2; identifier LOCAL_ERR; -expression ERRP; +identifier ERRP; idexpression V; typedef Error; -expression I; @@ + F(..., Error **ERRP) { ... - Error *LOCAL_ERR; ... when != LOCAL_ERR + when != ERRP ( - F2(ARGS, &LOCAL_ERR); - error_propagate(ERRP, LOCAL_ERR); diff --git a/scripts/coccinelle/return_directly.cocci b/scripts/coccinelle/return_directly.cocci index 7b0b6ac..c52f4fc 100644 --- a/scripts/coccinelle/return_directly.cocci +++ b/scripts/coccinelle/return_directly.cocci @@ -12,8 +12,10 @@ identifier F; ... - T VAR; ... when != VAR -- VAR = E; + +- VAR = ++ return + E; - return VAR; -+ return E; ... when != VAR } diff --git a/block.c b/block.c index c537307..ecca55a 100644 --- a/block.c +++ b/block.c @@ -294,12 +294,14 @@ typedef struct CreateCo { static void coroutine_fn bdrv_create_co_entry(void *opaque) { + Error *local_err = NULL; int ret; CreateCo *cco = opaque; assert(cco->drv); - ret = cco->drv->bdrv_create(cco->filename, cco->opts, &cco->err); + ret = cco->drv->bdrv_create(cco->filename, cco->opts, &local_err); + error_propagate(&cco->err, local_err); cco->ret = ret; } @@ -351,13 +353,17 @@ out: int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp) { BlockDriver *drv; + Error *local_err = NULL; + int ret; drv = bdrv_find_protocol(filename, true, errp); if (drv == NULL) { return -ENOENT; } - return bdrv_create(drv, filename, opts, errp); + ret = bdrv_create(drv, filename, opts, &local_err); + error_propagate(errp, local_err); + return ret; } /** diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index e0e5d9e..afc8d6b 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -155,7 +155,7 @@ static int l2_load(BlockDriverState *bs, uint64_t l2_offset, { BDRVQcow2State *s = bs->opaque; return qcow2_cache_get(bs, s->l2_table_cache, l2_offset, - (void **)l2_table); + (void **) l2_table); } /* diff --git a/blockdev.c b/blockdev.c index 3b6d242..028dba3 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3654,6 +3654,7 @@ void qmp_blockdev_mirror(const char *device, const char *target, BlockBackend *blk; BlockDriverState *target_bs; AioContext *aio_context; + Error *local_err = NULL; blk = blk_by_name(device); if (!blk) { @@ -3677,11 +3678,16 @@ void qmp_blockdev_mirror(const char *device, const char *target, bdrv_set_aio_context(target_bs, aio_context); - blockdev_mirror_common(bs, target_bs, has_replaces, replaces, sync, - has_speed, speed, has_granularity, granularity, - has_buf_size, buf_size, has_on_source_error, - on_source_error, has_on_target_error, - on_target_error, true, true, errp); + blockdev_mirror_common(bs, target_bs, + has_replaces, replaces, sync, + has_speed, speed, + has_granularity, granularity, + has_buf_size, buf_size, + has_on_source_error, on_source_error, + has_on_target_error, on_target_error, + true, true, + &local_err); + error_propagate(errp, local_err); aio_context_release(aio_context); } diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c index a0ccb61..ae40db8 100644 --- a/hw/ppc/spapr_vio.c +++ b/hw/ppc/spapr_vio.c @@ -57,6 +57,8 @@ static char *spapr_vio_get_dev_name(DeviceState *qdev) { VIOsPAPRDevice *dev = VIO_SPAPR_DEVICE(qdev); VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev); + + /* Device tree style name device@reg */ return g_strdup_printf("%s@%x", pc->dt_name, dev->reg); } diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c index 178c4e7..d177218 100644 --- a/hw/scsi/megasas.c +++ b/hw/scsi/megasas.c @@ -412,7 +412,12 @@ static uint64_t megasas_fw_time(void) struct tm curtime; qemu_get_timedate(&curtime, 0); - return ((uint64_t)curtime.tm_sec & 0xff) << 48 | ((uint64_t)curtime.tm_min & 0xff) << 40 | ((uint64_t)curtime.tm_hour & 0xff) << 32 | ((uint64_t)curtime.tm_mday & 0xff) << 24 | ((uint64_t)curtime.tm_mon & 0xff) << 16 | ((uint64_t)(curtime.tm_year + 1900) & 0xffff); + return ((uint64_t)curtime.tm_sec & 0xff) << 48 | + ((uint64_t)curtime.tm_min & 0xff) << 40 | + ((uint64_t)curtime.tm_hour & 0xff) << 32 | + ((uint64_t)curtime.tm_mday & 0xff) << 24 | + ((uint64_t)curtime.tm_mon & 0xff) << 16 | + ((uint64_t)(curtime.tm_year + 1900) & 0xffff); } /* diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c index c212990..f4e333e 100644 --- a/hw/timer/mc146818rtc.c +++ b/hw/timer/mc146818rtc.c @@ -107,7 +107,8 @@ static uint64_t get_guest_rtc_ns(RTCState *s) { uint64_t guest_clock = qemu_clock_get_ns(rtc_clock); - return s->base_rtc * NANOSECONDS_PER_SECOND + guest_clock - s->last_update + s->offset; + return s->base_rtc * NANOSECONDS_PER_SECOND + + guest_clock - s->last_update + s->offset; } #ifdef TARGET_I386 diff --git a/qga/commands-win32.c b/qga/commands-win32.c index b00a891..9c9be12 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -1163,7 +1163,8 @@ int64_t qmp_guest_get_time(Error **errp) return -1; } - return ((((int64_t)tf.dwHighDateTime << 32) | tf.dwLowDateTime) - W32_FT_OFFSET) * 100; + return ((((int64_t)tf.dwHighDateTime << 32) | tf.dwLowDateTime) + - W32_FT_OFFSET) * 100; } void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c index 7cbda0b..df7d220 100644 --- a/target-mips/dsp_helper.c +++ b/target-mips/dsp_helper.c @@ -3274,11 +3274,14 @@ target_ulong helper_dextr_l(target_ulong ac, target_ulong shift, CPUMIPSState *env) { uint64_t temp[3]; + target_ulong result; shift = shift & 0x3F; mipsdsp_rndrashift_acc(temp, ac, shift, env); - return (temp[1] << 63) | (temp[0] >> 1); + result = (temp[1] << 63) | (temp[0] >> 1); + + return result; } target_ulong helper_dextr_r_l(target_ulong ac, target_ulong shift, @@ -3286,6 +3289,7 @@ target_ulong helper_dextr_r_l(target_ulong ac, target_ulong shift, { uint64_t temp[3]; uint32_t temp128; + target_ulong result; shift = shift & 0x3F; mipsdsp_rndrashift_acc(temp, ac, shift, env); @@ -3305,7 +3309,9 @@ target_ulong helper_dextr_r_l(target_ulong ac, target_ulong shift, set_DSPControl_overflow_flag(1, 23, env); } - return (temp[1] << 63) | (temp[0] >> 1); + result = (temp[1] << 63) | (temp[0] >> 1); + + return result; } target_ulong helper_dextr_rs_l(target_ulong ac, target_ulong shift, @@ -3313,6 +3319,7 @@ target_ulong helper_dextr_rs_l(target_ulong ac, target_ulong shift, { uint64_t temp[3]; uint32_t temp128; + target_ulong result; shift = shift & 0x3F; mipsdsp_rndrashift_acc(temp, ac, shift, env); @@ -3338,7 +3345,9 @@ target_ulong helper_dextr_rs_l(target_ulong ac, target_ulong shift, } set_DSPControl_overflow_flag(1, 23, env); } - return (temp[1] << 63) | (temp[0] >> 1); + result = (temp[1] << 63) | (temp[0] >> 1); + + return result; } #endif diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c index 25310b5..6e8b83a 100644 --- a/ui/qemu-pixman.c +++ b/ui/qemu-pixman.c @@ -180,8 +180,10 @@ void qemu_pixman_linebuf_copy(pixman_image_t *fb, int width, int x, int y, pixman_image_t *qemu_pixman_mirror_create(pixman_format_code_t format, pixman_image_t *image) { - return pixman_image_create_bits(format, pixman_image_get_width(image), - pixman_image_get_height(image), NULL, + return pixman_image_create_bits(format, + pixman_image_get_width(image), + pixman_image_get_height(image), + NULL, pixman_image_get_stride(image)); } Eduardo Habkost (3): error: Remove NULL checks on error_propagate() calls error: Remove unnecessary local_err variables coccinelle: Remove unnecessary variables for function return value scripts/coccinelle/error_propagate_null.cocci | 10 +++++++ scripts/coccinelle/remove_local_err.cocci | 29 ++++++++++++++++++ scripts/coccinelle/return_directly.cocci | 21 ++++++++++++++ audio/audio.c | 10 ++----- block.c | 20 ++++--------- block/archipelago.c | 4 +-- block/qcow2-cluster.c | 7 ++--- block/qcow2-refcount.c | 7 ++--- block/qcow2.c | 4 +-- block/quorum.c | 4 +-- block/raw-posix.c | 24 +++------------ block/raw_bsd.c | 9 +----- block/rbd.c | 4 +-- block/snapshot.c | 4 +-- block/vmdk.c | 6 ++-- block/vvfat.c | 5 +--- blockdev.c | 12 ++------ bootdevice.c | 4 +-- dump.c | 4 +-- hw/acpi/aml-build.c | 13 ++------- hw/audio/intel-hda.c | 5 +--- hw/display/vga.c | 4 +-- hw/ide/qdev.c | 4 +-- hw/intc/s390_flic_kvm.c | 5 ++-- hw/net/ne2000-isa.c | 4 +-- hw/pci-host/uninorth.c | 5 +--- hw/ppc/spapr_vio.c | 5 +--- hw/s390x/s390-virtio-ccw.c | 5 +--- hw/s390x/virtio-ccw.c | 42 +++++---------------------- hw/scsi/megasas.c | 5 +--- hw/scsi/scsi-generic.c | 5 +--- hw/timer/mc146818rtc.c | 4 +-- hw/usb/dev-storage.c | 4 +-- hw/virtio/virtio-pci.c | 4 +-- linux-user/signal.c | 15 +++------- page_cache.c | 5 +--- qga/commands-posix.c | 4 +-- qga/commands-win32.c | 13 ++------- qobject/qlist.c | 5 +--- qom/object.c | 4 +-- target-i386/cpu.c | 4 +-- target-i386/fpu_helper.c | 10 ++----- target-i386/kvm.c | 5 ++-- target-mips/op_helper.c | 4 +-- target-s390x/helper.c | 6 +--- target-sparc/cc_helper.c | 25 ++++------------ target-tricore/op_helper.c | 13 +++------ tests/display-vga-test.c | 6 +--- tests/endianness-test.c | 5 +--- tests/i440fx-test.c | 4 +-- tests/intel-hda-test.c | 6 +--- tests/test-filter-redirector.c | 6 +--- tests/virtio-blk-test.c | 5 +--- tests/virtio-console-test.c | 6 +--- tests/virtio-net-test.c | 6 +--- tests/virtio-scsi-test.c | 6 +--- tests/wdt_ib700-test.c | 6 +--- ui/cursor.c | 10 ++----- ui/qemu-pixman.c | 13 ++++----- util/module.c | 6 +--- vl.c | 5 +--- 61 files changed, 159 insertions(+), 346 deletions(-) create mode 100644 scripts/coccinelle/error_propagate_null.cocci create mode 100644 scripts/coccinelle/remove_local_err.cocci create mode 100644 scripts/coccinelle/return_directly.cocci -- 2.5.5