qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] target-arm: fix build on fedora
@ 2013-12-23 11:56 Michael S. Tsirkin
  2013-12-23 12:37 ` Peter Maydell
  0 siblings, 1 reply; 11+ messages in thread
From: Michael S. Tsirkin @ 2013-12-23 11:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, John Rigby, lersek, Alexander Graf,
	Richard Henderson

commit 5ce4f35781028ce1aee3341e6002f925fdc7aaf3
    "target-arm: A64: add set_pc cpu method"

introduces an array aarch64_cpus which is zero
size if this code is built without CONFIG_USER_ONLY.
In particular an attempt to iterate over this array produces a warning:

 CC    aarch64-softmmu/target-arm/cpu64.o
/scm/qemu/target-arm/cpu64.c: In function ‘aarch64_cpu_register_types’:
/scm/qemu/target-arm/cpu64.c:124:5: error: comparison of unsigned
expression < 0 is always false [-Werror=type-limits]
     for (i = 0; i < ARRAY_SIZE(aarch64_cpus); i++) {
     ^
cc1: all warnings being treated as errors

This is the result of ARRAY_SIZE being an unsigned type,
causing i to be promoted to unsigned int as well.

As zero size arrays are a gcc extension, it seems
cleanest to add a dummy element with NULL name,
and test for it during registration.

Cc: Alexander Graf <agraf@suse.de>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

I have queued this in my tree since it prevents me from
being able to build and test properly.
Pls review and ack.

 target-arm/cpu64.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index 04ce879..2efe189 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -58,6 +58,7 @@ static const ARMCPUInfo aarch64_cpus[] = {
 #ifdef CONFIG_USER_ONLY
     { .name = "any",         .initfn = aarch64_any_initfn },
 #endif
+    { .name = NULL }
 };
 
 static void aarch64_cpu_initfn(Object *obj)
@@ -100,6 +101,10 @@ static void aarch64_cpu_register(const ARMCPUInfo *info)
         .class_init = info->class_init,
     };
 
+    if (!info->name) {
+        return;
+    }
+
     type_info.name = g_strdup_printf("%s-" TYPE_ARM_CPU, info->name);
     type_register(&type_info);
     g_free((void *)type_info.name);
-- 
MST

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: fix build on fedora
  2013-12-23 11:56 [Qemu-devel] [PATCH] target-arm: fix build on fedora Michael S. Tsirkin
@ 2013-12-23 12:37 ` Peter Maydell
  2013-12-23 12:50   ` Paolo Bonzini
                     ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Peter Maydell @ 2013-12-23 12:37 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: John Rigby, Richard Henderson, Laszlo Ersek, QEMU Developers,
	Alexander Graf

On 23 December 2013 11:56, Michael S. Tsirkin <mst@redhat.com> wrote:
> commit 5ce4f35781028ce1aee3341e6002f925fdc7aaf3
>     "target-arm: A64: add set_pc cpu method"
>
> introduces an array aarch64_cpus which is zero
> size if this code is built without CONFIG_USER_ONLY.
> In particular an attempt to iterate over this array produces a warning:
>
>  CC    aarch64-softmmu/target-arm/cpu64.o
> /scm/qemu/target-arm/cpu64.c: In function ‘aarch64_cpu_register_types’:
> /scm/qemu/target-arm/cpu64.c:124:5: error: comparison of unsigned
> expression < 0 is always false [-Werror=type-limits]
>      for (i = 0; i < ARRAY_SIZE(aarch64_cpus); i++) {
>      ^
> cc1: all warnings being treated as errors
>
> This is the result of ARRAY_SIZE being an unsigned type,
> causing i to be promoted to unsigned int as well.

I guess this is a new gcc warning, since this all builds
fine for me (gcc 4.6.3).

> As zero size arrays are a gcc extension, it seems
> cleanest to add a dummy element with NULL name,
> and test for it during registration.
>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> I have queued this in my tree since it prevents me from
> being able to build and test properly.
> Pls review and ack.
>
>  target-arm/cpu64.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
> index 04ce879..2efe189 100644
> --- a/target-arm/cpu64.c
> +++ b/target-arm/cpu64.c
> @@ -58,6 +58,7 @@ static const ARMCPUInfo aarch64_cpus[] = {
>  #ifdef CONFIG_USER_ONLY
>      { .name = "any",         .initfn = aarch64_any_initfn },
>  #endif
> +    { .name = NULL }
>  };
>
>  static void aarch64_cpu_initfn(Object *obj)
> @@ -100,6 +101,10 @@ static void aarch64_cpu_register(const ARMCPUInfo *info)
>          .class_init = info->class_init,
>      };
>
> +    if (!info->name) {
> +        return;
> +    }
> +
>      type_info.name = g_strdup_printf("%s-" TYPE_ARM_CPU, info->name);
>      type_register(&type_info);
>      g_free((void *)type_info.name);

At a minimum, if we take this approach we should add TODO comments
to the effect that the NULL terminator and the if() can be removed
when the first real AArch64 CPU is added.

I think I'd rather put the if (!info->name) continue into the function
which is doing the looping over the array.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: fix build on fedora
  2013-12-23 12:37 ` Peter Maydell
@ 2013-12-23 12:50   ` Paolo Bonzini
  2013-12-23 12:59     ` Peter Maydell
  2013-12-23 13:45   ` Andreas Färber
  2013-12-23 14:16   ` Michael S. Tsirkin
  2 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2013-12-23 12:50 UTC (permalink / raw)
  To: Peter Maydell
  Cc: John Rigby, Michael S. Tsirkin, Alexander Graf, QEMU Developers,
	Laszlo Ersek, Richard Henderson

Il 23/12/2013 13:37, Peter Maydell ha scritto:
> At a minimum, if we take this approach we should add TODO comments
> to the effect that the NULL terminator and the if() can be removed
> when the first real AArch64 CPU is added.
> 
> I think I'd rather put the if (!info->name) continue into the function
> which is doing the looping over the array.

Or just change the termination condition from a check on the array size
to one on info->name.

Paolo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: fix build on fedora
  2013-12-23 12:50   ` Paolo Bonzini
@ 2013-12-23 12:59     ` Peter Maydell
  2013-12-23 13:32       ` Stefan Weil
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2013-12-23 12:59 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: John Rigby, Michael S. Tsirkin, Alexander Graf, QEMU Developers,
	Laszlo Ersek, Richard Henderson

On 23 December 2013 12:50, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 23/12/2013 13:37, Peter Maydell ha scritto:
>> At a minimum, if we take this approach we should add TODO comments
>> to the effect that the NULL terminator and the if() can be removed
>> when the first real AArch64 CPU is added.
>>
>> I think I'd rather put the if (!info->name) continue into the function
>> which is doing the looping over the array.
>
> Or just change the termination condition from a check on the array size
> to one on info->name.

That would take it out of line with the equivalent 32 bit ARM code
(and also moxie and openrisc for what little that's worth) and be
fractionally more tedious to revert later.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: fix build on fedora
  2013-12-23 12:59     ` Peter Maydell
@ 2013-12-23 13:32       ` Stefan Weil
  2013-12-23 13:41         ` Peter Maydell
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Weil @ 2013-12-23 13:32 UTC (permalink / raw)
  To: Peter Maydell, Paolo Bonzini
  Cc: John Rigby, Michael S. Tsirkin, QEMU Developers, Alexander Graf,
	Laszlo Ersek, Richard Henderson

Am 23.12.2013 13:59, schrieb Peter Maydell:
> On 23 December 2013 12:50, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 23/12/2013 13:37, Peter Maydell ha scritto:
>>> At a minimum, if we take this approach we should add TODO comments
>>> to the effect that the NULL terminator and the if() can be removed
>>> when the first real AArch64 CPU is added.
>>>
>>> I think I'd rather put the if (!info->name) continue into the function
>>> which is doing the looping over the array.
>> Or just change the termination condition from a check on the array size
>> to one on info->name.
> That would take it out of line with the equivalent 32 bit ARM code
> (and also moxie and openrisc for what little that's worth) and be
> fractionally more tedious to revert later.
>
> thanks
> -- PMM

What about adding a dummy CPU (which can be removed later)?

I also got a warning here when I used cgcc / smatch recently, but did
not send a patch because there are too many possible fixes and none of
them seemed to be elegant :-)

Nevertheless, here is one of them:

--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -46,6 +46,11 @@ static void aarch64_any_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_V7MP);
     set_feature(&cpu->env, ARM_FEATURE_AARCH64);
 }
+#else
+static void aarch64_dummy_initfn(Object *obj)
+{
+    hw_error("Dummy CPU not supported");
+}
 #endif
 
 typedef struct ARMCPUInfo {
@@ -57,6 +62,9 @@ typedef struct ARMCPUInfo {
 static const ARMCPUInfo aarch64_cpus[] = {
 #ifdef CONFIG_USER_ONLY
     { .name = "any",         .initfn = aarch64_any_initfn },
+#else
+    /* TODO: Dummy CPU to avoid empty array. Fix when a real CPU is
added. */
+    { .name = "dummy",       .initfn = aarch64_dummy_initfn },
 #endif
 };

Of course, any other fix is also okay.

Cheers and Merry Christmas

Stefan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: fix build on fedora
  2013-12-23 13:32       ` Stefan Weil
@ 2013-12-23 13:41         ` Peter Maydell
  2013-12-23 14:15           ` Michael S. Tsirkin
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2013-12-23 13:41 UTC (permalink / raw)
  To: Stefan Weil
  Cc: Michael S. Tsirkin, QEMU Developers, Alexander Graf,
	Paolo Bonzini, Laszlo Ersek, Richard Henderson

On 23 December 2013 13:32, Stefan Weil <sw@weilnetz.de> wrote:
> Am 23.12.2013 13:59, schrieb Peter Maydell:
>> On 23 December 2013 12:50, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> Il 23/12/2013 13:37, Peter Maydell ha scritto:
>>>> At a minimum, if we take this approach we should add TODO comments
>>>> to the effect that the NULL terminator and the if() can be removed
>>>> when the first real AArch64 CPU is added.
>>>>
>>>> I think I'd rather put the if (!info->name) continue into the function
>>>> which is doing the looping over the array.
>>> Or just change the termination condition from a check on the array size
>>> to one on info->name.
>> That would take it out of line with the equivalent 32 bit ARM code
>> (and also moxie and openrisc for what little that's worth) and be
>> fractionally more tedious to revert later.

> What about adding a dummy CPU (which can be removed later)?

That would be user-visible, which seems a bad thing.
I agree that there aren't any fantastic solutions here;
I think something more or less like Michael's patch with
a TODO note so it's easy for me to take it out again when
I add an actual A57 emulation in a couple of months will
do. This is just a temporary thing since at the moment we
only support -cpu any for userspace and -cpu host for KVM.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: fix build on fedora
  2013-12-23 12:37 ` Peter Maydell
  2013-12-23 12:50   ` Paolo Bonzini
@ 2013-12-23 13:45   ` Andreas Färber
  2013-12-23 13:56     ` Peter Maydell
  2013-12-23 14:16   ` Michael S. Tsirkin
  2 siblings, 1 reply; 11+ messages in thread
From: Andreas Färber @ 2013-12-23 13:45 UTC (permalink / raw)
  To: Peter Maydell
  Cc: John Rigby, Michael S. Tsirkin, Alexander Graf, QEMU Developers,
	Laszlo Ersek, Richard Henderson

Am 23.12.2013 13:37, schrieb Peter Maydell:
> On 23 December 2013 11:56, Michael S. Tsirkin <mst@redhat.com> wrote:
>> commit 5ce4f35781028ce1aee3341e6002f925fdc7aaf3
>>     "target-arm: A64: add set_pc cpu method"
>>
>> introduces an array aarch64_cpus which is zero
>> size if this code is built without CONFIG_USER_ONLY.
>> In particular an attempt to iterate over this array produces a warning:
>>
>>  CC    aarch64-softmmu/target-arm/cpu64.o
>> /scm/qemu/target-arm/cpu64.c: In function ‘aarch64_cpu_register_types’:
>> /scm/qemu/target-arm/cpu64.c:124:5: error: comparison of unsigned
>> expression < 0 is always false [-Werror=type-limits]
>>      for (i = 0; i < ARRAY_SIZE(aarch64_cpus); i++) {
>>      ^
>> cc1: all warnings being treated as errors
>>
>> This is the result of ARRAY_SIZE being an unsigned type,
>> causing i to be promoted to unsigned int as well.
> 
> I guess this is a new gcc warning, since this all builds
> fine for me (gcc 4.6.3).

No problem noticed with 4.8.1 on today's master either.

>> As zero size arrays are a gcc extension, it seems
>> cleanest to add a dummy element with NULL name,
>> and test for it during registration.
>>
>> Cc: Alexander Graf <agraf@suse.de>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> Cc: Richard Henderson <rth@twiddle.net>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> ---
>>
>> I have queued this in my tree since it prevents me from
>> being able to build and test properly.
>> Pls review and ack.
>>
>>  target-arm/cpu64.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
>> index 04ce879..2efe189 100644
>> --- a/target-arm/cpu64.c
>> +++ b/target-arm/cpu64.c
>> @@ -58,6 +58,7 @@ static const ARMCPUInfo aarch64_cpus[] = {
>>  #ifdef CONFIG_USER_ONLY
>>      { .name = "any",         .initfn = aarch64_any_initfn },
>>  #endif
>> +    { .name = NULL }
>>  };
>>
>>  static void aarch64_cpu_initfn(Object *obj)
>> @@ -100,6 +101,10 @@ static void aarch64_cpu_register(const ARMCPUInfo *info)
>>          .class_init = info->class_init,
>>      };
>>
>> +    if (!info->name) {
>> +        return;
>> +    }
>> +
>>      type_info.name = g_strdup_printf("%s-" TYPE_ARM_CPU, info->name);
>>      type_register(&type_info);
>>      g_free((void *)type_info.name);
> 
> At a minimum, if we take this approach we should add TODO comments
> to the effect that the NULL terminator and the if() can be removed
> when the first real AArch64 CPU is added.
> 
> I think I'd rather put the if (!info->name) continue into the function
> which is doing the looping over the array.

While I share your sentiment wrt this workaround, what's the status of
getting a real 64-bit CPU applied? Isn't the Cortex-A57/A53 CPU
independent of whether we have all MPCore etc. pieces in place? That
would seem the most elegant solution to me, even if not "usable" yet.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: fix build on fedora
  2013-12-23 13:45   ` Andreas Färber
@ 2013-12-23 13:56     ` Peter Maydell
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2013-12-23 13:56 UTC (permalink / raw)
  To: Andreas Färber
  Cc: John Rigby, Michael S. Tsirkin, Alexander Graf, QEMU Developers,
	Laszlo Ersek, Richard Henderson

On 23 December 2013 13:45, Andreas Färber <afaerber@suse.de> wrote:
> While I share your sentiment wrt this workaround, what's the status of
> getting a real 64-bit CPU applied? Isn't the Cortex-A57/A53 CPU
> independent of whether we have all MPCore etc. pieces in place? That
> would seem the most elegant solution to me, even if not "usable" yet.

It's next on my todo list after we get the usermode instruction
emulation wrapped up, so should be sometime in the next
few months, as part of the system emulation work.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: fix build on fedora
  2013-12-23 14:15           ` Michael S. Tsirkin
@ 2013-12-23 14:15             ` Peter Maydell
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2013-12-23 14:15 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Stefan Weil, QEMU Developers, Alexander Graf, Paolo Bonzini,
	Laszlo Ersek, Richard Henderson

On 23 December 2013 14:15, Michael S. Tsirkin <mst@redhat.com> wrote:
> OK, ack my patch then? I'll add a comment
> /* TODO: remove when we support more CPUs. */

Post an updated patch (with TODO comments in both
places) and I'll ack it.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: fix build on fedora
  2013-12-23 13:41         ` Peter Maydell
@ 2013-12-23 14:15           ` Michael S. Tsirkin
  2013-12-23 14:15             ` Peter Maydell
  0 siblings, 1 reply; 11+ messages in thread
From: Michael S. Tsirkin @ 2013-12-23 14:15 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Stefan Weil, QEMU Developers, Alexander Graf, Paolo Bonzini,
	Laszlo Ersek, Richard Henderson

On Mon, Dec 23, 2013 at 01:41:50PM +0000, Peter Maydell wrote:
> On 23 December 2013 13:32, Stefan Weil <sw@weilnetz.de> wrote:
> > Am 23.12.2013 13:59, schrieb Peter Maydell:
> >> On 23 December 2013 12:50, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >>> Il 23/12/2013 13:37, Peter Maydell ha scritto:
> >>>> At a minimum, if we take this approach we should add TODO comments
> >>>> to the effect that the NULL terminator and the if() can be removed
> >>>> when the first real AArch64 CPU is added.
> >>>>
> >>>> I think I'd rather put the if (!info->name) continue into the function
> >>>> which is doing the looping over the array.
> >>> Or just change the termination condition from a check on the array size
> >>> to one on info->name.
> >> That would take it out of line with the equivalent 32 bit ARM code
> >> (and also moxie and openrisc for what little that's worth) and be
> >> fractionally more tedious to revert later.
> 
> > What about adding a dummy CPU (which can be removed later)?
> 
> That would be user-visible, which seems a bad thing.
> I agree that there aren't any fantastic solutions here;
> I think something more or less like Michael's patch with
> a TODO note so it's easy for me to take it out again when
> I add an actual A57 emulation in a couple of months will
> do. This is just a temporary thing since at the moment we
> only support -cpu any for userspace and -cpu host for KVM.
> 
> thanks
> -- PMM

OK, ack my patch then? I'll add a comment
/* TODO: remove when we support more CPUs. */

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: fix build on fedora
  2013-12-23 12:37 ` Peter Maydell
  2013-12-23 12:50   ` Paolo Bonzini
  2013-12-23 13:45   ` Andreas Färber
@ 2013-12-23 14:16   ` Michael S. Tsirkin
  2 siblings, 0 replies; 11+ messages in thread
From: Michael S. Tsirkin @ 2013-12-23 14:16 UTC (permalink / raw)
  To: Peter Maydell
  Cc: John Rigby, Richard Henderson, Laszlo Ersek, QEMU Developers,
	Alexander Graf

On Mon, Dec 23, 2013 at 12:37:41PM +0000, Peter Maydell wrote:
> On 23 December 2013 11:56, Michael S. Tsirkin <mst@redhat.com> wrote:
> > commit 5ce4f35781028ce1aee3341e6002f925fdc7aaf3
> >     "target-arm: A64: add set_pc cpu method"
> >
> > introduces an array aarch64_cpus which is zero
> > size if this code is built without CONFIG_USER_ONLY.
> > In particular an attempt to iterate over this array produces a warning:
> >
> >  CC    aarch64-softmmu/target-arm/cpu64.o
> > /scm/qemu/target-arm/cpu64.c: In function ‘aarch64_cpu_register_types’:
> > /scm/qemu/target-arm/cpu64.c:124:5: error: comparison of unsigned
> > expression < 0 is always false [-Werror=type-limits]
> >      for (i = 0; i < ARRAY_SIZE(aarch64_cpus); i++) {
> >      ^
> > cc1: all warnings being treated as errors
> >
> > This is the result of ARRAY_SIZE being an unsigned type,
> > causing i to be promoted to unsigned int as well.
> 
> I guess this is a new gcc warning, since this all builds
> fine for me (gcc 4.6.3).

I see this with gcc 4.8.2 on Fedora 19.

> > As zero size arrays are a gcc extension, it seems
> > cleanest to add a dummy element with NULL name,
> > and test for it during registration.
> >
> > Cc: Alexander Graf <agraf@suse.de>
> > Cc: Peter Maydell <peter.maydell@linaro.org>
> > Cc: Richard Henderson <rth@twiddle.net>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >
> > I have queued this in my tree since it prevents me from
> > being able to build and test properly.
> > Pls review and ack.
> >
> >  target-arm/cpu64.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
> > index 04ce879..2efe189 100644
> > --- a/target-arm/cpu64.c
> > +++ b/target-arm/cpu64.c
> > @@ -58,6 +58,7 @@ static const ARMCPUInfo aarch64_cpus[] = {
> >  #ifdef CONFIG_USER_ONLY
> >      { .name = "any",         .initfn = aarch64_any_initfn },
> >  #endif
> > +    { .name = NULL }
> >  };
> >
> >  static void aarch64_cpu_initfn(Object *obj)
> > @@ -100,6 +101,10 @@ static void aarch64_cpu_register(const ARMCPUInfo *info)
> >          .class_init = info->class_init,
> >      };
> >
> > +    if (!info->name) {
> > +        return;
> > +    }
> > +
> >      type_info.name = g_strdup_printf("%s-" TYPE_ARM_CPU, info->name);
> >      type_register(&type_info);
> >      g_free((void *)type_info.name);
> 
> At a minimum, if we take this approach we should add TODO comments
> to the effect that the NULL terminator and the if() can be removed
> when the first real AArch64 CPU is added.
> 
> I think I'd rather put the if (!info->name) continue into the function
> which is doing the looping over the array.
> 
> thanks
> -- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-12-23 14:16 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-23 11:56 [Qemu-devel] [PATCH] target-arm: fix build on fedora Michael S. Tsirkin
2013-12-23 12:37 ` Peter Maydell
2013-12-23 12:50   ` Paolo Bonzini
2013-12-23 12:59     ` Peter Maydell
2013-12-23 13:32       ` Stefan Weil
2013-12-23 13:41         ` Peter Maydell
2013-12-23 14:15           ` Michael S. Tsirkin
2013-12-23 14:15             ` Peter Maydell
2013-12-23 13:45   ` Andreas Färber
2013-12-23 13:56     ` Peter Maydell
2013-12-23 14:16   ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).