Hello Philippe
On Wed, Apr 1, 2020 at 7:26 PM Philippe Mathieu-Daudé <phi...@redhat.com>
wrote:
> Hi Simran,
>
> On 4/1/20 2:11 PM, Simran Singhal wrote:
> > Compress two lines into a single line if immediate return statement is
> found.
>
> How did you find these changes? Manual audit, some tool?
>
I wrote coccinelle script to do these changes.
> >
> > It also remove variables progress, val, data, ret and sock
> > as they are no longer needed.
> >
> > Remove space between function "mixer_load" and '(' to fix the
> > checkpatch.pl error:-
> > ERROR: space prohibited between function name and open parenthesis '('
> >
> > Signed-off-by: Simran Singhal <singhalsimr...@gmail.com>
> > ---
> > block/file-posix.c | 3 +--
> > block/nfs.c | 3 +--
> > block/nvme.c | 4 +---
> > block/vhdx.c | 3 +--
> > hw/audio/ac97.c | 4 +---
> > hw/audio/adlib.c | 5 +----
> > hw/display/cirrus_vga.c | 4 +---
> > migration/ram.c | 4 +---
> > ui/gtk.c | 3 +--
> > util/qemu-sockets.c | 5 +----
> > 10 files changed, 10 insertions(+), 28 deletions(-)
> >
> > diff --git a/block/file-posix.c b/block/file-posix.c
> > index 7e19bbff5f..dc01f0d4d3 100644
> > --- a/block/file-posix.c
> > +++ b/block/file-posix.c
> > @@ -1627,8 +1627,7 @@ static int handle_aiocb_write_zeroes_unmap(void
> *opaque)
> >
> > /* If we couldn't manage to unmap while guaranteed that the area
> reads as
> > * all-zero afterwards, just write zeroes without unmapping */
> > - ret = handle_aiocb_write_zeroes(aiocb);
> > - return ret;
> > + return handle_aiocb_write_zeroes(aiocb);
> > }
> >
> > #ifndef HAVE_COPY_FILE_RANGE
> > diff --git a/block/nfs.c b/block/nfs.c
> > index cc2413d5ab..100f15bd1f 100644
> > --- a/block/nfs.c
> > +++ b/block/nfs.c
> > @@ -623,8 +623,7 @@ static int nfs_file_open(BlockDriverState *bs, QDict
> *options, int flags,
> > }
> >
> > bs->total_sectors = ret;
> > - ret = 0;
> > - return ret;
> > + return 0;
> > }
> >
> > static QemuOptsList nfs_create_opts = {
> > diff --git a/block/nvme.c b/block/nvme.c
> > index 7b7c0cc5d6..eb2f54dd9d 100644
> > --- a/block/nvme.c
> > +++ b/block/nvme.c
> > @@ -575,11 +575,9 @@ static bool nvme_poll_cb(void *opaque)
> > {
> > EventNotifier *e = opaque;
> > BDRVNVMeState *s = container_of(e, BDRVNVMeState, irq_notifier);
> > - bool progress = false;
> >
> > trace_nvme_poll_cb(s);
> > - progress = nvme_poll_queues(s);
> > - return progress;
> > + return nvme_poll_queues(s);
> > }
> >
> > static int nvme_init(BlockDriverState *bs, const char *device, int
> namespace,
> > diff --git a/block/vhdx.c b/block/vhdx.c
> > index 33e57cd656..2c0e7ee44d 100644
> > --- a/block/vhdx.c
> > +++ b/block/vhdx.c
> > @@ -411,8 +411,7 @@ int vhdx_update_headers(BlockDriverState *bs,
> BDRVVHDXState *s,
> > if (ret < 0) {
> > return ret;
> > }
> > - ret = vhdx_update_header(bs, s, generate_data_write_guid, log_guid);
> > - return ret;
> > + return vhdx_update_header(bs, s, generate_data_write_guid,
> log_guid);
> > }
> >
> > /* opens the specified header block from the VHDX file header section
> */
> > diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
> > index 1ec87feec0..8a9b9924c4 100644
> > --- a/hw/audio/ac97.c
> > +++ b/hw/audio/ac97.c
> > @@ -573,11 +573,9 @@ static uint32_t nam_readb (void *opaque, uint32_t
> addr)
> > static uint32_t nam_readw (void *opaque, uint32_t addr)
> > {
> > AC97LinkState *s = opaque;
> > - uint32_t val = ~0U;
> > uint32_t index = addr;
>
> Why remove 'val' and keep 'index'?
>
I focused on doing one type of change in this patch. I removed 'val'
because that variable was no longer useful with these changes.
> > s->cas = 0;
> > - val = mixer_load (s, index);
> > - return val;
> > + return mixer_load(s, index);
> > }
> >
> > static uint32_t nam_readl (void *opaque, uint32_t addr)
> > diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
> > index d6c1fb0586..7c3b67dcfb 100644
> > --- a/hw/audio/adlib.c
> > +++ b/hw/audio/adlib.c
> > @@ -120,13 +120,10 @@ static void adlib_write(void *opaque, uint32_t
> nport, uint32_t val)
> > static uint32_t adlib_read(void *opaque, uint32_t nport)
> > {
> > AdlibState *s = opaque;
> > - uint8_t data;
> > int a = nport & 3;
>
> Why remove 'data' and keep 'a'?
>
I focused on doing one type of change in this patch. I removed 'data'
because that variable was no longer useful with these changes.
>
> >
> > adlib_kill_timers (s);
> > - data = OPLRead (s->opl, a);
> > -
> > - return data;
> > + return OPLRead (s->opl, a);
> > }
> >
> > static void timer_handler (void *opaque, int c, double interval_Sec)
> > diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> > index 0d391e1300..1f29731ffe 100644
> > --- a/hw/display/cirrus_vga.c
> > +++ b/hw/display/cirrus_vga.c
> > @@ -2411,12 +2411,10 @@ static uint64_t cirrus_linear_bitblt_read(void
> *opaque,
> > unsigned size)
> > {
> > CirrusVGAState *s = opaque;
> > - uint32_t ret;
>
> Why remove 'ret' and keep 's'?
>
I focused on doing one type of change in this patch. I removed 'ret'
because that variable was no longer useful with these changes.
>
> >
> > /* XXX handle bitblt */
> > (void)s;
> > - ret = 0xff;
> > - return ret;
> > + return 0xff;
> > }
> >
> > static void cirrus_linear_bitblt_write(void *opaque,
> > diff --git a/migration/ram.c b/migration/ram.c
> > index 04f13feb2e..06cba88632 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -2135,9 +2135,7 @@ int
> ram_postcopy_send_discard_bitmap(MigrationState *ms)
> > }
> > trace_ram_postcopy_send_discard_bitmap();
> >
> > - ret = postcopy_each_ram_send_discard(ms);
> > -
> > - return ret;
> > + return postcopy_each_ram_send_discard(ms);
> > }
> >
> > /**
> > diff --git a/ui/gtk.c b/ui/gtk.c
> > index 030b251c61..83f2f5d49b 100644
> > --- a/ui/gtk.c
> > +++ b/ui/gtk.c
> > @@ -1650,8 +1650,7 @@ static GSList *gd_vc_menu_init(GtkDisplayState *s,
> VirtualConsole *vc,
> > G_CALLBACK(gd_menu_switch_vc), s);
> > gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), vc->menu_item);
> >
> > - group =
> gtk_radio_menu_item_get_group(GTK_RADIO_MENU_ITEM(vc->menu_item));
> > - return group;
> > + return
> gtk_radio_menu_item_get_group(GTK_RADIO_MENU_ITEM(vc->menu_item));
> > }
> >
> > #if defined(CONFIG_VTE)
> > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> > index bcc06d0e01..86c48b9fa5 100644
> > --- a/util/qemu-sockets.c
> > +++ b/util/qemu-sockets.c
> > @@ -765,15 +765,12 @@ static int vsock_connect_addr(const struct
> sockaddr_vm *svm, Error **errp)
> > static int vsock_connect_saddr(VsockSocketAddress *vaddr, Error **errp)
> > {
> > struct sockaddr_vm svm;
> > - int sock = -1;
> >
> > if (!vsock_parse_vaddr_to_sockaddr(vaddr, &svm, errp)) {
> > return -1;
> > }
> >
> > - sock = vsock_connect_addr(&svm, errp);
> > -
> > - return sock;
> > + return vsock_connect_addr(&svm, errp);
> > }
> >
> > static int vsock_listen_saddr(VsockSocketAddress *vaddr,
> >
>
>
