On Mon, Dec 23, 2013 at 12:37:41PM +0000, Peter Maydell wrote:
> On 23 December 2013 11:56, Michael S. Tsirkin <m...@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 <ag...@suse.de>
> > Cc: Peter Maydell <peter.mayd...@linaro.org>
> > Cc: Richard Henderson <r...@twiddle.net>
> > Signed-off-by: Michael S. Tsirkin <m...@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
