From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:43003)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <tommusta@gmail.com>) id 1XPFBc-0001zD-AG
	for qemu-devel@nongnu.org; Wed, 03 Sep 2014 14:21:49 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <tommusta@gmail.com>) id 1XPFBT-0005Ev-8z
	for qemu-devel@nongnu.org; Wed, 03 Sep 2014 14:21:40 -0400
Message-ID: <54075C23.6060507@gmail.com>
Date: Wed, 03 Sep 2014 13:21:23 -0500
From: Tom Musta <tommusta@gmail.com>
MIME-Version: 1.0
References: <1409246113-6519-1-git-send-email-pbonzini@redhat.com>
	<1409246113-6519-5-git-send-email-pbonzini@redhat.com>
In-Reply-To: <1409246113-6519-5-git-send-email-pbonzini@redhat.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 04/17] ppc: use ARRAY_SIZE in gdbstub.c
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org
Cc: dgibson@redhat.com, qemu-ppc@nongnu.org

On 8/28/2014 12:15 PM, Paolo Bonzini wrote:
> Match the idiom used by linux-user/signal.c and
> linux-user/elfload.c.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  target-ppc/gdbstub.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target-ppc/gdbstub.c b/target-ppc/gdbstub.c
> index 14675f4..bad49ae 100644
> --- a/target-ppc/gdbstub.c
> +++ b/target-ppc/gdbstub.c
> @@ -138,7 +138,7 @@ int ppc_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
>              {
>                  uint32_t cr = 0;
>                  int i;
> -                for (i = 0; i < 8; i++) {
> +                for (i = 0; i < ARRAY_SIZE(env->crf); i++) {
>                      cr |= env->crf[i] << (32 - ((i + 1) * 4));
>                  }
>                  gdb_get_reg32(mem_buf, cr);
> @@ -246,7 +246,7 @@ int ppc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>              {
>                  uint32_t cr = ldl_p(mem_buf);
>                  int i;
> -                for (i = 0; i < 8; i++) {
> +                for (i = 0; i < ARRAY_SIZE(env->crf); i++) {
>                      env->crf[i] = (cr >> (32 - ((i + 1) * 4))) & 0xF;
>                  }
>                  break;
> 

Since the same code appears in 3 different places, would it be better to implement a reusable function in target-ppc/cpu.h?

I.e.:

static inline uint32_t ppc_get_cr(const CPUPPCState *env) {
    uint32_t cr = 0;
    for (i = 0; i < ARRAY_SIZE(env->crf); i++) {
        cr |= ppc_get_crf(env, i) << (32 - ((i + 1) * 4));
    }
    return cr;
}