Re: [Qemu-devel] [PATCH 4/9] address_space_write_continue: Distill common code

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2975 bytes --]

On Thu, Jul 07, 2016 at 04:33:38PM -0700, Andrey Smirnov wrote:
> Move call to memory_region_dispatch_write() outside of swtich statement
> since the only thing that is different about all of those call is
> "length" argument and that matches value in "l". Also collapse switch
> statement to occupy less vertical space.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

I'm not a big fan of putting multiple statements on a single line,
especially flow control like break.  But apart from that,

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  exec.c | 31 +++++++------------------------
>  1 file changed, 7 insertions(+), 24 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index c26bcea..eea3018 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2566,33 +2566,16 @@ static MemTxResult address_space_write_continue(AddressSpace *as, hwaddr addr,
>              /* XXX: could force current_cpu to NULL to avoid
>                 potential bugs */
>              switch (l) {
> -            case 8:
> -                /* 64 bit write access */
> -                val = ldq_p(buf);
> -                result |= memory_region_dispatch_write(mr, addr1, val, 8,
> -                                                       attrs);
> -                break;
> -            case 4:
> -                /* 32 bit write access */
> -                val = ldl_p(buf);
> -                result |= memory_region_dispatch_write(mr, addr1, val, 4,
> -                                                       attrs);
> -                break;
> -            case 2:
> -                /* 16 bit write access */
> -                val = lduw_p(buf);
> -                result |= memory_region_dispatch_write(mr, addr1, val, 2,
> -                                                       attrs);
> -                break;
> -            case 1:
> -                /* 8 bit write access */
> -                val = ldub_p(buf);
> -                result |= memory_region_dispatch_write(mr, addr1, val, 1,
> -                                                       attrs);
> -                break;
> +            case 8: val = ldq_p(buf);  break; /* 64 bit write access */
> +            case 4: val = ldl_p(buf);  break; /* 32 bit write access */
> +            case 2: val = lduw_p(buf); break; /* 16 bit write access */
> +            case 1: val = ldub_p(buf); break; /*  8 bit write access */
>              default:
>                  abort();
>              }
> +
> +            result |= memory_region_dispatch_write(mr, addr1, val, l,
> +                                                   attrs);
>          } else {
>              /* RAM case */
>              ptr = qemu_map_ram_ptr(mr->ram_block, addr1);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]