On 06/10/2016 02:12 PM, Eduardo Habkost wrote:
> Use Coccinelle script to replace 'ret = E; return ret' with
> 'return E'. The script will do the substitution only when the
> function return type and variable type are the same.
>
> Sending as RFC because the patch looks more intrusive than the
> others. Probably better to split it per subsystem and let each
> maintainer review and apply it?
Borderline on size, so yeah, splitting it across several subsystems may
ease review (although then the patch will be committed in piecemeal
fashion, and you'd have to ensure the script/coccinelle/ patch goes in
first...)
At any rate, it's fairly mechanical, so I'll review it as is:
>
> Signed-off-by: Eduardo Habkost
> ---
> 47 files changed, 90 insertions(+), 254 deletions(-)
> create mode 100644 scripts/coccinelle/return_directly.cocci
Nice diffstat.
> +++ b/block/qcow2-cluster.c
> @@ -154,11 +154,8 @@ static int l2_load(BlockDriverState *bs, uint64_t
> l2_offset,
> uint64_t **l2_table)
> {
> BDRVQcow2State *s = bs->opaque;
> -int ret;
> -
> -ret = qcow2_cache_get(bs, s->l2_table_cache, l2_offset, (void**)
> l2_table);
> -
> -return ret;
> +return qcow2_cache_get(bs, s->l2_table_cache, l2_offset,
> + (void **)l2_table);
Coccinelle changed spacing of the cast. I don't care strongly enough to
require a touchup if this is the only thing, but may be worth fixing if
you have to respin (for example to split up by submaintainers).
> +++ b/block/raw_bsd.c
> @@ -190,10 +190,7 @@ static int raw_has_zero_init(BlockDriverState *bs)
>
> static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
> {
> -int ret;
> -
> -ret = bdrv_create_file(filename, opts, errp);
> -return ret;
> +return bdrv_create_file(filename, opts, errp);
> }
Potential followup patch: delete raw_create(), and:
- .bdrv_create = &raw_create,
+ .bdrv_create = bdrv_create_file,
but doesn't affect this patch.
> +++ b/block/rbd.c
> @@ -875,10 +875,7 @@ static int qemu_rbd_snap_rollback(BlockDriverState *bs,
>const char *snapshot_name)
> {
> BDRVRBDState *s = bs->opaque;
> -int r;
> -
> -r = rbd_snap_rollback(s->image, snapshot_name);
> -return r;
> +return rbd_snap_rollback(s->image, snapshot_name);
Coccinelle lost the blank line between declarations and statements;
might be nice to manually touch that up and add it back in.
> +++ b/hw/ppc/spapr_vio.c
> @@ -57,12 +57,7 @@ static char *spapr_vio_get_dev_name(DeviceState *qdev)
> {
> VIOsPAPRDevice *dev = VIO_SPAPR_DEVICE(qdev);
> VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
> -char *name;
> -
> -/* Device tree style name device@reg */
> -name = g_strdup_printf("%s@%x", pc->dt_name, dev->reg);
> -
> -return name;
> +return g_strdup_printf("%s@%x", pc->dt_name, dev->reg);
Coccinelle lost the comment; might be worth keeping it.
> +++ b/hw/scsi/megasas.c
> @@ -410,17 +410,9 @@ static void megasas_encode_lba(uint8_t *cdb, uint64_t
> lba,
> static uint64_t megasas_fw_time(void)
> {
> struct tm curtime;
> -uint64_t bcd_time;
>
> qemu_get_timedate(&curtime, 0);
> -bcd_time = ((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) & 0x);
> -
> -return bcd_time;
> +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) & 0x);
Eww. Coccinelle botched that formatting. You'll need to manually fix
this one.
> +++ b/hw/timer/mc146818rtc.c
> @@ -105,12 +105,9 @@ static inline bool rtc_running(RTCState *s)
>
> static uint64_t get_guest_rtc_ns(RTCState *s)
> {
> -uint64_t guest_rtc;
> uint64_t guest_clock = qemu_clock_get_ns(rtc_clock);
>
> -guest_rtc = s->base_rtc * NANOSECONDS_PER_SECOND +
> -guest_clock - s->last_update + s->offset;
> -return guest_rtc;
> +return s->base_rtc * NANOSECONDS_PER_SECOND + guest_clock -
> s->last_update + s->offset;
> }
Worth wrapping that line again (not as bad as the megasas one, though).
> +++ b/qga/commands-win32.c
> @@ -1150,7 +1150,6 @@ out:
> int64_t qmp_guest_get_time(Error **errp)
> {
> SYSTEMTIME ts = {0};
> -int64_t time_ns;
> FILETIME tf;
>
> GetSystemTime(&ts);
> @@ -1164,10 +1163,7 @@ int64_t qmp_guest_get_time(Error **errp)
> return -1;
> }
>
> -time_ns = int64_t)tf.dwHighDateTime << 32) | tf.dwLowDateTime)
> -- W32_FT_OFFSET) * 100;
> -
> -return time_ns;
> +return int64_t)tf.
