1. Patchew
  2. QEMU
  3. View series

[PATCH] or1k: Fix compilation hiccup

Eric Blake posted 1 patch 5 years, 4 months ago
Test docker-mingw@fedora passed
Test checkpatch failed
Test asan passed
Test docker-quick@centos7 passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200526185132.1652355-1-eblake@redhat.com
hw/openrisc/openrisc_sim.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] or1k: Fix compilation hiccup
Posted by Eric Blake 5 years, 4 months ago 
On my Fedora 32 machine, gcc 10.1.1 at -O2 (the default for a bare
'./configure') has a false-positive complaint:

  CC      or1k-softmmu/hw/openrisc/openrisc_sim.o
/home/eblake/qemu/hw/openrisc/openrisc_sim.c: In function ‘openrisc_sim_init’:
/home/eblake/qemu/hw/openrisc/openrisc_sim.c:87:42: error: ‘cpu_irqs[0]’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
   87 |         sysbus_connect_irq(s, i, cpu_irqs[i][irq_pin]);
      |                                  ~~~~~~~~^~~

Initializing both pointers of cpu_irqs[] to NULL is sufficient to shut
up the compiler, even though they are definitely assigned in
openrisc_sim_init() prior to the inlined call to
openrisc_sim_ompic_init() containing the line in question.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 hw/openrisc/openrisc_sim.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index d08ce6181199..95011a8015b4 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -129,7 +129,7 @@ static void openrisc_sim_init(MachineState *machine)
     const char *kernel_filename = machine->kernel_filename;
     OpenRISCCPU *cpu = NULL;
     MemoryRegion *ram;
-    qemu_irq *cpu_irqs[2];
+    qemu_irq *cpu_irqs[2] = {};
     qemu_irq serial_irq;
     int n;
     unsigned int smp_cpus = machine->smp.cpus;
-- 
2.26.2
Re: [PATCH] or1k: Fix compilation hiccup
Posted by no-reply@patchew.org 5 years, 4 months ago 
Patchew URL: https://patchew.org/QEMU/20200526185132.1652355-1-eblake@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20200526185132.1652355-1-eblake@redhat.com
Subject: [PATCH] or1k: Fix compilation hiccup
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
d96d2fb or1k: Fix compilation hiccup

=== OUTPUT BEGIN ===
ERROR: spaces required around that '*' (ctx:WxV)
#33: FILE: hw/openrisc/openrisc_sim.c:132:
+    qemu_irq *cpu_irqs[2] = {};
              ^

total: 1 errors, 0 warnings, 8 lines checked

Commit d96d2fbbc5db (or1k: Fix compilation hiccup) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200526185132.1652355-1-eblake@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH] or1k: Fix compilation hiccup
Posted by Eric Blake 5 years, 4 months ago 
On 5/26/20 6:21 PM, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20200526185132.1652355-1-eblake@redhat.com/
> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 

> === OUTPUT BEGIN ===
> ERROR: spaces required around that '*' (ctx:WxV)
> #33: FILE: hw/openrisc/openrisc_sim.c:132:
> +    qemu_irq *cpu_irqs[2] = {};
>                ^
> 
> total: 1 errors, 0 warnings, 8 lines checked
> 
> Commit d96d2fbbc5db (or1k: Fix compilation hiccup) has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.

False positive, due to 'qemu_irq' not following the normal naming 
conventions for typedefs.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org
Re: [PATCH] or1k: Fix compilation hiccup
Posted by Thomas Huth 5 years, 4 months ago 
On 26/05/2020 20.51, Eric Blake wrote:
> On my Fedora 32 machine, gcc 10.1.1 at -O2 (the default for a bare
> './configure') has a false-positive complaint:
> 
>   CC      or1k-softmmu/hw/openrisc/openrisc_sim.o
> /home/eblake/qemu/hw/openrisc/openrisc_sim.c: In function ‘openrisc_sim_init’:
> /home/eblake/qemu/hw/openrisc/openrisc_sim.c:87:42: error: ‘cpu_irqs[0]’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    87 |         sysbus_connect_irq(s, i, cpu_irqs[i][irq_pin]);
>       |                                  ~~~~~~~~^~~
> 
> Initializing both pointers of cpu_irqs[] to NULL is sufficient to shut
> up the compiler, even though they are definitely assigned in
> openrisc_sim_init() prior to the inlined call to
> openrisc_sim_ompic_init() containing the line in question.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  hw/openrisc/openrisc_sim.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
> index d08ce6181199..95011a8015b4 100644
> --- a/hw/openrisc/openrisc_sim.c
> +++ b/hw/openrisc/openrisc_sim.c
> @@ -129,7 +129,7 @@ static void openrisc_sim_init(MachineState *machine)
>      const char *kernel_filename = machine->kernel_filename;
>      OpenRISCCPU *cpu = NULL;
>      MemoryRegion *ram;
> -    qemu_irq *cpu_irqs[2];
> +    qemu_irq *cpu_irqs[2] = {};
>      qemu_irq serial_irq;
>      int n;
>      unsigned int smp_cpus = machine->smp.cpus;
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>
Re: [PATCH] or1k: Fix compilation hiccup
Posted by Philippe Mathieu-Daudé 5 years, 4 months ago 
On 5/26/20 8:51 PM, Eric Blake wrote:
> On my Fedora 32 machine, gcc 10.1.1 at -O2 (the default for a bare
> './configure') has a false-positive complaint:
> 
>   CC      or1k-softmmu/hw/openrisc/openrisc_sim.o
> /home/eblake/qemu/hw/openrisc/openrisc_sim.c: In function ‘openrisc_sim_init’:
> /home/eblake/qemu/hw/openrisc/openrisc_sim.c:87:42: error: ‘cpu_irqs[0]’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    87 |         sysbus_connect_irq(s, i, cpu_irqs[i][irq_pin]);
>       |                                  ~~~~~~~~^~~
> 
> Initializing both pointers of cpu_irqs[] to NULL is sufficient to shut
> up the compiler, even though they are definitely assigned in
> openrisc_sim_init() prior to the inlined call to
> openrisc_sim_ompic_init() containing the line in question.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  hw/openrisc/openrisc_sim.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
> index d08ce6181199..95011a8015b4 100644
> --- a/hw/openrisc/openrisc_sim.c
> +++ b/hw/openrisc/openrisc_sim.c
> @@ -129,7 +129,7 @@ static void openrisc_sim_init(MachineState *machine)
>      const char *kernel_filename = machine->kernel_filename;
>      OpenRISCCPU *cpu = NULL;
>      MemoryRegion *ram;
> -    qemu_irq *cpu_irqs[2];
> +    qemu_irq *cpu_irqs[2] = {};

Ah I now remembered why this warning sound familiar:
https://bugs.launchpad.net/qemu/+bug/1874073

Peter suggested a different fix, I tested it, and was expecting Martin
Liska to post an updated patch.

>      qemu_irq serial_irq;
>      int n;
>      unsigned int smp_cpus = machine->smp.cpus;
>
Re: [PATCH] or1k: Fix compilation hiccup
Posted by Christophe de Dinechin 5 years, 4 months ago 
On 2020-05-26 at 20:51 CEST, Eric Blake wrote...
> On my Fedora 32 machine, gcc 10.1.1 at -O2 (the default for a bare
> './configure') has a false-positive complaint:
>
>   CC      or1k-softmmu/hw/openrisc/openrisc_sim.o
> /home/eblake/qemu/hw/openrisc/openrisc_sim.c: In function ‘openrisc_sim_init’:
> /home/eblake/qemu/hw/openrisc/openrisc_sim.c:87:42: error: ‘cpu_irqs[0]’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    87 |         sysbus_connect_irq(s, i, cpu_irqs[i][irq_pin]);
>       |                                  ~~~~~~~~^~~
>
> Initializing both pointers of cpu_irqs[] to NULL is sufficient to shut
> up the compiler, even though they are definitely assigned in
> openrisc_sim_init() prior to the inlined call to
> openrisc_sim_ompic_init() containing the line in question.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  hw/openrisc/openrisc_sim.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
> index d08ce6181199..95011a8015b4 100644
> --- a/hw/openrisc/openrisc_sim.c
> +++ b/hw/openrisc/openrisc_sim.c
> @@ -129,7 +129,7 @@ static void openrisc_sim_init(MachineState *machine)
>      const char *kernel_filename = machine->kernel_filename;
>      OpenRISCCPU *cpu = NULL;
>      MemoryRegion *ram;
> -    qemu_irq *cpu_irqs[2];
> +    qemu_irq *cpu_irqs[2] = {};

Why is the value [2] correct here? The loop that initializes loops over
machine->smp.cpus. Is it always less than 2 on this machine?


>      qemu_irq serial_irq;
>      int n;
>      unsigned int smp_cpus = machine->smp.cpus;


--
Cheers,
Christophe de Dinechin (IRC c3d)
Re: [PATCH] or1k: Fix compilation hiccup
Posted by Peter Maydell 5 years, 4 months ago 
On Fri, 29 May 2020 at 17:23, Christophe de Dinechin
<dinechin@redhat.com> wrote:
> On 2020-05-26 at 20:51 CEST, Eric Blake wrote...
> > diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
> > index d08ce6181199..95011a8015b4 100644
> > --- a/hw/openrisc/openrisc_sim.c
> > +++ b/hw/openrisc/openrisc_sim.c
> > @@ -129,7 +129,7 @@ static void openrisc_sim_init(MachineState *machine)
> >      const char *kernel_filename = machine->kernel_filename;
> >      OpenRISCCPU *cpu = NULL;
> >      MemoryRegion *ram;
> > -    qemu_irq *cpu_irqs[2];
> > +    qemu_irq *cpu_irqs[2] = {};
>
> Why is the value [2] correct here? The loop that initializes loops over
> machine->smp.cpus. Is it always less than 2 on this machine?

Yes: openrisc_sim_machine_init() sets mc->max_cpus = 2.
My suggestion of adding an assert() is essentially telling the
compiler that indeed smp_cpus must always be in the range [1,2],
which we can tell but it can't.

thanks
-- PMM
Re: [PATCH] or1k: Fix compilation hiccup
Posted by Markus Armbruster 5 years, 3 months ago 
Peter Maydell <peter.maydell@linaro.org> writes:

> On Fri, 29 May 2020 at 17:23, Christophe de Dinechin
> <dinechin@redhat.com> wrote:
>> On 2020-05-26 at 20:51 CEST, Eric Blake wrote...
>> > diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
>> > index d08ce6181199..95011a8015b4 100644
>> > --- a/hw/openrisc/openrisc_sim.c
>> > +++ b/hw/openrisc/openrisc_sim.c
>> > @@ -129,7 +129,7 @@ static void openrisc_sim_init(MachineState *machine)
>> >      const char *kernel_filename = machine->kernel_filename;
>> >      OpenRISCCPU *cpu = NULL;
>> >      MemoryRegion *ram;
>> > -    qemu_irq *cpu_irqs[2];
>> > +    qemu_irq *cpu_irqs[2] = {};
>>
>> Why is the value [2] correct here? The loop that initializes loops over
>> machine->smp.cpus. Is it always less than 2 on this machine?
>
> Yes: openrisc_sim_machine_init() sets mc->max_cpus = 2.
> My suggestion of adding an assert() is essentially telling the
> compiler that indeed smp_cpus must always be in the range [1,2],
> which we can tell but it can't.

Do we have a proper patch for this on the list?
Re: [PATCH] or1k: Fix compilation hiccup
Posted by Markus Armbruster 5 years, 3 months ago 
Markus Armbruster <armbru@redhat.com> writes:

> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> On Fri, 29 May 2020 at 17:23, Christophe de Dinechin
>> <dinechin@redhat.com> wrote:
>>> On 2020-05-26 at 20:51 CEST, Eric Blake wrote...
>>> > diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
>>> > index d08ce6181199..95011a8015b4 100644
>>> > --- a/hw/openrisc/openrisc_sim.c
>>> > +++ b/hw/openrisc/openrisc_sim.c
>>> > @@ -129,7 +129,7 @@ static void openrisc_sim_init(MachineState *machine)
>>> >      const char *kernel_filename = machine->kernel_filename;
>>> >      OpenRISCCPU *cpu = NULL;
>>> >      MemoryRegion *ram;
>>> > -    qemu_irq *cpu_irqs[2];
>>> > +    qemu_irq *cpu_irqs[2] = {};
>>>
>>> Why is the value [2] correct here? The loop that initializes loops over
>>> machine->smp.cpus. Is it always less than 2 on this machine?
>>
>> Yes: openrisc_sim_machine_init() sets mc->max_cpus = 2.
>> My suggestion of adding an assert() is essentially telling the
>> compiler that indeed smp_cpus must always be in the range [1,2],
>> which we can tell but it can't.
>
> Do we have a proper patch for this on the list?

Apparently not.

Philippe did try Peter's suggestion, found it works, but then posted it
only to Launchpad.  Philippe, please post to the list, so we can finally
get this fixed.
Re: [PATCH] or1k: Fix compilation hiccup
Posted by Philippe Mathieu-Daudé 5 years, 3 months ago 
On 6/8/20 8:03 AM, Markus Armbruster wrote:
> Markus Armbruster <armbru@redhat.com> writes:
> 
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>>> On Fri, 29 May 2020 at 17:23, Christophe de Dinechin
>>> <dinechin@redhat.com> wrote:
>>>> On 2020-05-26 at 20:51 CEST, Eric Blake wrote...
>>>>> diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
>>>>> index d08ce6181199..95011a8015b4 100644
>>>>> --- a/hw/openrisc/openrisc_sim.c
>>>>> +++ b/hw/openrisc/openrisc_sim.c
>>>>> @@ -129,7 +129,7 @@ static void openrisc_sim_init(MachineState *machine)
>>>>>      const char *kernel_filename = machine->kernel_filename;
>>>>>      OpenRISCCPU *cpu = NULL;
>>>>>      MemoryRegion *ram;
>>>>> -    qemu_irq *cpu_irqs[2];
>>>>> +    qemu_irq *cpu_irqs[2] = {};
>>>>
>>>> Why is the value [2] correct here? The loop that initializes loops over
>>>> machine->smp.cpus. Is it always less than 2 on this machine?
>>>
>>> Yes: openrisc_sim_machine_init() sets mc->max_cpus = 2.
>>> My suggestion of adding an assert() is essentially telling the
>>> compiler that indeed smp_cpus must always be in the range [1,2],
>>> which we can tell but it can't.
>>
>> Do we have a proper patch for this on the list?
> 
> Apparently not.
> 
> Philippe did try Peter's suggestion, found it works, but then posted it
> only to Launchpad.  Philippe, please post to the list, so we can finally
> get this fixed.

Sorry since it was Eric finding, I didn't understood I had to post it.
Will do.
Re: [PATCH] or1k: Fix compilation hiccup
Posted by Markus Armbruster 5 years, 3 months ago 
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 6/8/20 8:03 AM, Markus Armbruster wrote:
>> Markus Armbruster <armbru@redhat.com> writes:
>> 
>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>
>>>> On Fri, 29 May 2020 at 17:23, Christophe de Dinechin
>>>> <dinechin@redhat.com> wrote:
>>>>> On 2020-05-26 at 20:51 CEST, Eric Blake wrote...
>>>>>> diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
>>>>>> index d08ce6181199..95011a8015b4 100644
>>>>>> --- a/hw/openrisc/openrisc_sim.c
>>>>>> +++ b/hw/openrisc/openrisc_sim.c
>>>>>> @@ -129,7 +129,7 @@ static void openrisc_sim_init(MachineState *machine)
>>>>>>      const char *kernel_filename = machine->kernel_filename;
>>>>>>      OpenRISCCPU *cpu = NULL;
>>>>>>      MemoryRegion *ram;
>>>>>> -    qemu_irq *cpu_irqs[2];
>>>>>> +    qemu_irq *cpu_irqs[2] = {};
>>>>>
>>>>> Why is the value [2] correct here? The loop that initializes loops over
>>>>> machine->smp.cpus. Is it always less than 2 on this machine?
>>>>
>>>> Yes: openrisc_sim_machine_init() sets mc->max_cpus = 2.
>>>> My suggestion of adding an assert() is essentially telling the
>>>> compiler that indeed smp_cpus must always be in the range [1,2],
>>>> which we can tell but it can't.
>>>
>>> Do we have a proper patch for this on the list?
>> 
>> Apparently not.
>> 
>> Philippe did try Peter's suggestion, found it works, but then posted it
>> only to Launchpad.  Philippe, please post to the list, so we can finally
>> get this fixed.
>
> Sorry since it was Eric finding, I didn't understood I had to post it.
> Will do.

You didn't *have* to, but it'll help if you do :)
Re: [PATCH] or1k: Fix compilation hiccup
Posted by Eric Blake 5 years, 3 months ago 
On 6/8/20 4:15 AM, Markus Armbruster wrote:

>>>>> Yes: openrisc_sim_machine_init() sets mc->max_cpus = 2.
>>>>> My suggestion of adding an assert() is essentially telling the
>>>>> compiler that indeed smp_cpus must always be in the range [1,2],
>>>>> which we can tell but it can't.
>>>>
>>>> Do we have a proper patch for this on the list?
>>>
>>> Apparently not.
>>>
>>> Philippe did try Peter's suggestion, found it works, but then posted it
>>> only to Launchpad.  Philippe, please post to the list, so we can finally
>>> get this fixed.
>>
>> Sorry since it was Eric finding, I didn't understood I had to post it.
>> Will do.
> 
> You didn't *have* to, but it'll help if you do :)

Now at https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg01779.html

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.