[PATCH for-4.2] hw/i386: Fix compiler warning when CONFIG_IDE_ISA is disabled

[PATCH for-4.2] hw/i386: Fix compiler warning when CONFIG_IDE_ISA is disabled

[Thomas Huth]

When CONFIG_IDE_ISA is disabled, compilation currently fails:

 hw/i386/pc_piix.c: In function ‘pc_init1’:
 hw/i386/pc_piix.c:81:9: error: unused variable ‘i’ [-Werror=unused-variable]

Move the variable declaration to the right code block to avoid
this problem.

Fixes: 4501d317b50e ("hw/i386/pc: Extract pc_i8259_create()")
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/i386/pc_piix.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 2aefa3b8df..d187db761c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -78,7 +78,6 @@ static void pc_init1(MachineState *machine,
     X86MachineState *x86ms = X86_MACHINE(machine);
     MemoryRegion *system_memory = get_system_memory();
     MemoryRegion *system_io = get_system_io();
-    int i;
     PCIBus *pci_bus;
     ISABus *isa_bus;
     PCII440FXState *i440fx_state;
@@ -253,7 +252,7 @@ static void pc_init1(MachineState *machine,
     }
 #ifdef CONFIG_IDE_ISA
 else {
-        for(i = 0; i < MAX_IDE_BUS; i++) {
+        for (int i = 0; i < MAX_IDE_BUS; i++) {
             ISADevice *dev;
             char busname[] = "ide.0";
             dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
-- 
2.23.0

Re: [PATCH for-4.2] hw/i386: Fix compiler warning when CONFIG_IDE_ISA is disabled

[Michael S. Tsirkin]

On Fri, Nov 15, 2019 at 03:50:49PM +0100, Thomas Huth wrote:
> When CONFIG_IDE_ISA is disabled, compilation currently fails:
> 
>  hw/i386/pc_piix.c: In function ‘pc_init1’:
>  hw/i386/pc_piix.c:81:9: error: unused variable ‘i’ [-Werror=unused-variable]
> 
> Move the variable declaration to the right code block to avoid
> this problem.
> 
> Fixes: 4501d317b50e ("hw/i386/pc: Extract pc_i8259_create()")
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/i386/pc_piix.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 2aefa3b8df..d187db761c 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -78,7 +78,6 @@ static void pc_init1(MachineState *machine,
>      X86MachineState *x86ms = X86_MACHINE(machine);
>      MemoryRegion *system_memory = get_system_memory();
>      MemoryRegion *system_io = get_system_io();
> -    int i;
>      PCIBus *pci_bus;
>      ISABus *isa_bus;
>      PCII440FXState *i440fx_state;
> @@ -253,7 +252,7 @@ static void pc_init1(MachineState *machine,
>      }
>  #ifdef CONFIG_IDE_ISA
>  else {
> -        for(i = 0; i < MAX_IDE_BUS; i++) {
> +        for (int i = 0; i < MAX_IDE_BUS; i++) {
>              ISADevice *dev;
>              char busname[] = "ide.0";
>              dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
> -- 
> 2.23.0

Re: [PATCH for-4.2] hw/i386: Fix compiler warning when CONFIG_IDE_ISA is disabled

[Peter Maydell]

On Fri, 15 Nov 2019 at 15:10, Thomas Huth <thuth@redhat.com> wrote:
>
> When CONFIG_IDE_ISA is disabled, compilation currently fails:
>
>  hw/i386/pc_piix.c: In function ‘pc_init1’:
>  hw/i386/pc_piix.c:81:9: error: unused variable ‘i’ [-Werror=unused-variable]
>
> Move the variable declaration to the right code block to avoid
> this problem.
>
> Fixes: 4501d317b50e ("hw/i386/pc: Extract pc_i8259_create()")
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/i386/pc_piix.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 2aefa3b8df..d187db761c 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -78,7 +78,6 @@ static void pc_init1(MachineState *machine,
>      X86MachineState *x86ms = X86_MACHINE(machine);
>      MemoryRegion *system_memory = get_system_memory();
>      MemoryRegion *system_io = get_system_io();
> -    int i;
>      PCIBus *pci_bus;
>      ISABus *isa_bus;
>      PCII440FXState *i440fx_state;
> @@ -253,7 +252,7 @@ static void pc_init1(MachineState *machine,
>      }
>  #ifdef CONFIG_IDE_ISA
>  else {
> -        for(i = 0; i < MAX_IDE_BUS; i++) {
> +        for (int i = 0; i < MAX_IDE_BUS; i++) {
>              ISADevice *dev;
>              char busname[] = "ide.0";
>              dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],

Don't put variable declarations inside 'for' statements,
please. They should go at the start of a {} block.

thanks
-- PMM

Re: [PATCH for-4.2] hw/i386: Fix compiler warning when CONFIG_IDE_ISA is disabled

[Thomas Huth]

On 15/11/2019 16.54, Peter Maydell wrote:
> On Fri, 15 Nov 2019 at 15:10, Thomas Huth <thuth@redhat.com> wrote:
>>
>> When CONFIG_IDE_ISA is disabled, compilation currently fails:
>>
>>  hw/i386/pc_piix.c: In function ‘pc_init1’:
>>  hw/i386/pc_piix.c:81:9: error: unused variable ‘i’ [-Werror=unused-variable]
>>
>> Move the variable declaration to the right code block to avoid
>> this problem.
>>
>> Fixes: 4501d317b50e ("hw/i386/pc: Extract pc_i8259_create()")
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  hw/i386/pc_piix.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 2aefa3b8df..d187db761c 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -78,7 +78,6 @@ static void pc_init1(MachineState *machine,
>>      X86MachineState *x86ms = X86_MACHINE(machine);
>>      MemoryRegion *system_memory = get_system_memory();
>>      MemoryRegion *system_io = get_system_io();
>> -    int i;
>>      PCIBus *pci_bus;
>>      ISABus *isa_bus;
>>      PCII440FXState *i440fx_state;
>> @@ -253,7 +252,7 @@ static void pc_init1(MachineState *machine,
>>      }
>>  #ifdef CONFIG_IDE_ISA
>>  else {
>> -        for(i = 0; i < MAX_IDE_BUS; i++) {
>> +        for (int i = 0; i < MAX_IDE_BUS; i++) {
>>              ISADevice *dev;
>>              char busname[] = "ide.0";
>>              dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
> 
> Don't put variable declarations inside 'for' statements,
> please. They should go at the start of a {} block.

Why? We're using -std=gnu99 now, so this should not be an issue anymore.

 Thomas

Re: [PATCH for-4.2] hw/i386: Fix compiler warning when CONFIG_IDE_ISA is disabled

[Paolo Bonzini]

On 15/11/19 16:54, Thomas Huth wrote:
> On 15/11/2019 16.54, Peter Maydell wrote:
>> On Fri, 15 Nov 2019 at 15:10, Thomas Huth <thuth@redhat.com> wrote:
>>>
>>> When CONFIG_IDE_ISA is disabled, compilation currently fails:
>>>
>>>  hw/i386/pc_piix.c: In function ‘pc_init1’:
>>>  hw/i386/pc_piix.c:81:9: error: unused variable ‘i’ [-Werror=unused-variable]
>>>
>>> Move the variable declaration to the right code block to avoid
>>> this problem.
>>>
>>> Fixes: 4501d317b50e ("hw/i386/pc: Extract pc_i8259_create()")
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>  hw/i386/pc_piix.c | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>>> index 2aefa3b8df..d187db761c 100644
>>> --- a/hw/i386/pc_piix.c
>>> +++ b/hw/i386/pc_piix.c
>>> @@ -78,7 +78,6 @@ static void pc_init1(MachineState *machine,
>>>      X86MachineState *x86ms = X86_MACHINE(machine);
>>>      MemoryRegion *system_memory = get_system_memory();
>>>      MemoryRegion *system_io = get_system_io();
>>> -    int i;
>>>      PCIBus *pci_bus;
>>>      ISABus *isa_bus;
>>>      PCII440FXState *i440fx_state;
>>> @@ -253,7 +252,7 @@ static void pc_init1(MachineState *machine,
>>>      }
>>>  #ifdef CONFIG_IDE_ISA
>>>  else {
>>> -        for(i = 0; i < MAX_IDE_BUS; i++) {
>>> +        for (int i = 0; i < MAX_IDE_BUS; i++) {
>>>              ISADevice *dev;
>>>              char busname[] = "ide.0";
>>>              dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
>>
>> Don't put variable declarations inside 'for' statements,
>> please. They should go at the start of a {} block.
> 
> Why? We're using -std=gnu99 now, so this should not be an issue anymore.

For now I can squash the following while we discuss coding standards. :)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index fa62244f4d..0130b8fb4e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -244,7 +244,8 @@ static void pc_init1(MachineState *machine,
     }
 #ifdef CONFIG_IDE_ISA
 else {
-        for (int i = 0; i < MAX_IDE_BUS; i++) {
+        int i;
+        for (i = 0; i < MAX_IDE_BUS; i++) {
             ISADevice *dev;
             char busname[] = "ide.0";
             dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],



Paolo

Re: [PATCH for-4.2] hw/i386: Fix compiler warning when CONFIG_IDE_ISA is disabled

[Thomas Huth]

On 15/11/2019 17.13, Paolo Bonzini wrote:
> On 15/11/19 16:54, Thomas Huth wrote:
>> On 15/11/2019 16.54, Peter Maydell wrote:
>>> On Fri, 15 Nov 2019 at 15:10, Thomas Huth <thuth@redhat.com> wrote:
>>>>
>>>> When CONFIG_IDE_ISA is disabled, compilation currently fails:
>>>>
>>>>  hw/i386/pc_piix.c: In function ‘pc_init1’:
>>>>  hw/i386/pc_piix.c:81:9: error: unused variable ‘i’ [-Werror=unused-variable]
>>>>
>>>> Move the variable declaration to the right code block to avoid
>>>> this problem.
>>>>
>>>> Fixes: 4501d317b50e ("hw/i386/pc: Extract pc_i8259_create()")
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>  hw/i386/pc_piix.c | 3 +--
>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>>>> index 2aefa3b8df..d187db761c 100644
>>>> --- a/hw/i386/pc_piix.c
>>>> +++ b/hw/i386/pc_piix.c
>>>> @@ -78,7 +78,6 @@ static void pc_init1(MachineState *machine,
>>>>      X86MachineState *x86ms = X86_MACHINE(machine);
>>>>      MemoryRegion *system_memory = get_system_memory();
>>>>      MemoryRegion *system_io = get_system_io();
>>>> -    int i;
>>>>      PCIBus *pci_bus;
>>>>      ISABus *isa_bus;
>>>>      PCII440FXState *i440fx_state;
>>>> @@ -253,7 +252,7 @@ static void pc_init1(MachineState *machine,
>>>>      }
>>>>  #ifdef CONFIG_IDE_ISA
>>>>  else {
>>>> -        for(i = 0; i < MAX_IDE_BUS; i++) {
>>>> +        for (int i = 0; i < MAX_IDE_BUS; i++) {
>>>>              ISADevice *dev;
>>>>              char busname[] = "ide.0";
>>>>              dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
>>>
>>> Don't put variable declarations inside 'for' statements,
>>> please. They should go at the start of a {} block.
>>
>> Why? We're using -std=gnu99 now, so this should not be an issue anymore.
> 
> For now I can squash the following while we discuss coding standards. :)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index fa62244f4d..0130b8fb4e 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -244,7 +244,8 @@ static void pc_init1(MachineState *machine,
>      }
>  #ifdef CONFIG_IDE_ISA
>  else {
> -        for (int i = 0; i < MAX_IDE_BUS; i++) {
> +        int i;
> +        for (i = 0; i < MAX_IDE_BUS; i++) {
>              ISADevice *dev;
>              char busname[] = "ide.0";
>              dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],

Yes, please do. I guess we won't update CODING_STYLE.rst during the hard
freeze anymore ;-)

 Thomas

Re: [PATCH for-4.2] hw/i386: Fix compiler warning when CONFIG_IDE_ISA is disabled

[Philippe Mathieu-Daudé]

On 11/15/19 5:13 PM, Thomas Huth wrote:
> On 15/11/2019 17.13, Paolo Bonzini wrote:
>> On 15/11/19 16:54, Thomas Huth wrote:
>>> On 15/11/2019 16.54, Peter Maydell wrote:
>>>> On Fri, 15 Nov 2019 at 15:10, Thomas Huth <thuth@redhat.com> wrote:
>>>>>
>>>>> When CONFIG_IDE_ISA is disabled, compilation currently fails:
>>>>>
>>>>>   hw/i386/pc_piix.c: In function ‘pc_init1’:
>>>>>   hw/i386/pc_piix.c:81:9: error: unused variable ‘i’ [-Werror=unused-variable]
>>>>>
>>>>> Move the variable declaration to the right code block to avoid
>>>>> this problem.
>>>>>
>>>>> Fixes: 4501d317b50e ("hw/i386/pc: Extract pc_i8259_create()")
>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>> ---
>>>>>   hw/i386/pc_piix.c | 3 +--
>>>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>>>>> index 2aefa3b8df..d187db761c 100644
>>>>> --- a/hw/i386/pc_piix.c
>>>>> +++ b/hw/i386/pc_piix.c
>>>>> @@ -78,7 +78,6 @@ static void pc_init1(MachineState *machine,
>>>>>       X86MachineState *x86ms = X86_MACHINE(machine);
>>>>>       MemoryRegion *system_memory = get_system_memory();
>>>>>       MemoryRegion *system_io = get_system_io();
>>>>> -    int i;
>>>>>       PCIBus *pci_bus;
>>>>>       ISABus *isa_bus;
>>>>>       PCII440FXState *i440fx_state;
>>>>> @@ -253,7 +252,7 @@ static void pc_init1(MachineState *machine,
>>>>>       }
>>>>>   #ifdef CONFIG_IDE_ISA
>>>>>   else {
>>>>> -        for(i = 0; i < MAX_IDE_BUS; i++) {
>>>>> +        for (int i = 0; i < MAX_IDE_BUS; i++) {
>>>>>               ISADevice *dev;
>>>>>               char busname[] = "ide.0";
>>>>>               dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
>>>>
>>>> Don't put variable declarations inside 'for' statements,
>>>> please. They should go at the start of a {} block.
>>>
>>> Why? We're using -std=gnu99 now, so this should not be an issue anymore.
>>
>> For now I can squash the following while we discuss coding standards. :)

Thanks.

>>
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index fa62244f4d..0130b8fb4e 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -244,7 +244,8 @@ static void pc_init1(MachineState *machine,
>>       }
>>   #ifdef CONFIG_IDE_ISA
>>   else {
>> -        for (int i = 0; i < MAX_IDE_BUS; i++) {
>> +        int i;
>> +        for (i = 0; i < MAX_IDE_BUS; i++) {
>>               ISADevice *dev;
>>               char busname[] = "ide.0";
>>               dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
> 
> Yes, please do. I guess we won't update CODING_STYLE.rst during the hard
> freeze anymore ;-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH for-4.2] hw/i386: Fix compiler warning when CONFIG_IDE_ISA is disabled

[Peter Maydell]

On Fri, 15 Nov 2019 at 16:08, Thomas Huth <thuth@redhat.com> wrote:
>
> On 15/11/2019 16.54, Peter Maydell wrote:
> > On Fri, 15 Nov 2019 at 15:10, Thomas Huth <thuth@redhat.com> wrote:
> >> --- a/hw/i386/pc_piix.c
> >> +++ b/hw/i386/pc_piix.c
> >> @@ -78,7 +78,6 @@ static void pc_init1(MachineState *machine,
> >>      X86MachineState *x86ms = X86_MACHINE(machine);
> >>      MemoryRegion *system_memory = get_system_memory();
> >>      MemoryRegion *system_io = get_system_io();
> >> -    int i;
> >>      PCIBus *pci_bus;
> >>      ISABus *isa_bus;
> >>      PCII440FXState *i440fx_state;
> >> @@ -253,7 +252,7 @@ static void pc_init1(MachineState *machine,
> >>      }
> >>  #ifdef CONFIG_IDE_ISA
> >>  else {
> >> -        for(i = 0; i < MAX_IDE_BUS; i++) {
> >> +        for (int i = 0; i < MAX_IDE_BUS; i++) {
> >>              ISADevice *dev;
> >>              char busname[] = "ide.0";
> >>              dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
> >
> > Don't put variable declarations inside 'for' statements,
> > please. They should go at the start of a {} block.
>
> Why? We're using -std=gnu99 now, so this should not be an issue anymore.

Consistency with the rest of the code base, which mostly
avoids this particular trick. See the 'Declarations' section
of CODING_STYLE.rst.

As Paolo points out, there's a nice convenient block
here already, so there's not much to be gained from
putting the declaration in the middle of the for statement.

thanks
-- PMM

Re: [PATCH for-4.2] hw/i386: Fix compiler warning when CONFIG_IDE_ISA is disabled

[Thomas Huth]

On 15/11/2019 17.15, Peter Maydell wrote:
> On Fri, 15 Nov 2019 at 16:08, Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 15/11/2019 16.54, Peter Maydell wrote:
>>> On Fri, 15 Nov 2019 at 15:10, Thomas Huth <thuth@redhat.com> wrote:
>>>> --- a/hw/i386/pc_piix.c
>>>> +++ b/hw/i386/pc_piix.c
>>>> @@ -78,7 +78,6 @@ static void pc_init1(MachineState *machine,
>>>>      X86MachineState *x86ms = X86_MACHINE(machine);
>>>>      MemoryRegion *system_memory = get_system_memory();
>>>>      MemoryRegion *system_io = get_system_io();
>>>> -    int i;
>>>>      PCIBus *pci_bus;
>>>>      ISABus *isa_bus;
>>>>      PCII440FXState *i440fx_state;
>>>> @@ -253,7 +252,7 @@ static void pc_init1(MachineState *machine,
>>>>      }
>>>>  #ifdef CONFIG_IDE_ISA
>>>>  else {
>>>> -        for(i = 0; i < MAX_IDE_BUS; i++) {
>>>> +        for (int i = 0; i < MAX_IDE_BUS; i++) {
>>>>              ISADevice *dev;
>>>>              char busname[] = "ide.0";
>>>>              dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
>>>
>>> Don't put variable declarations inside 'for' statements,
>>> please. They should go at the start of a {} block.
>>
>> Why? We're using -std=gnu99 now, so this should not be an issue anymore.
> 
> Consistency with the rest of the code base, which mostly
> avoids this particular trick.

We've also got a few spots that use it...
(run e.g.: grep -r 'for (int ' hw/* )

> See the 'Declarations' section of CODING_STYLE.rst.

OK, that's a point. But since this gnu99 is a rather new option that we
just introduced less than a year ago, we should maybe think of whether
we want to allow this for for-loops now, too (since IMHO it's quite a
nice feature in gnu99).

 Thomas

Re: [PATCH for-4.2] hw/i386: Fix compiler warning when CONFIG_IDE_ISA is disabled

[Philippe Mathieu-Daudé]

On 11/15/19 5:12 PM, Thomas Huth wrote:
> On 15/11/2019 17.15, Peter Maydell wrote:
>> On Fri, 15 Nov 2019 at 16:08, Thomas Huth <thuth@redhat.com> wrote:
>>>
>>> On 15/11/2019 16.54, Peter Maydell wrote:
>>>> On Fri, 15 Nov 2019 at 15:10, Thomas Huth <thuth@redhat.com> wrote:
>>>>> --- a/hw/i386/pc_piix.c
>>>>> +++ b/hw/i386/pc_piix.c
>>>>> @@ -78,7 +78,6 @@ static void pc_init1(MachineState *machine,
>>>>>       X86MachineState *x86ms = X86_MACHINE(machine);
>>>>>       MemoryRegion *system_memory = get_system_memory();
>>>>>       MemoryRegion *system_io = get_system_io();
>>>>> -    int i;
>>>>>       PCIBus *pci_bus;
>>>>>       ISABus *isa_bus;
>>>>>       PCII440FXState *i440fx_state;
>>>>> @@ -253,7 +252,7 @@ static void pc_init1(MachineState *machine,
>>>>>       }
>>>>>   #ifdef CONFIG_IDE_ISA
>>>>>   else {
>>>>> -        for(i = 0; i < MAX_IDE_BUS; i++) {
>>>>> +        for (int i = 0; i < MAX_IDE_BUS; i++) {
>>>>>               ISADevice *dev;
>>>>>               char busname[] = "ide.0";
>>>>>               dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
>>>>
>>>> Don't put variable declarations inside 'for' statements,
>>>> please. They should go at the start of a {} block.
>>>
>>> Why? We're using -std=gnu99 now, so this should not be an issue anymore.
>>
>> Consistency with the rest of the code base, which mostly
>> avoids this particular trick.
> 
> We've also got a few spots that use it...
> (run e.g.: grep -r 'for (int ' hw/* )

~31 (vs 8000+):

$ git grep -E 'for\s*\((int|size_t)'|egrep -v '\.(cc|cpp):'
audio/audio_legacy.c:400:        for (int i = 0; audio_prio_list[i]; i++) {
hw/block/pflash_cfi02.c:281:    for (int i = 0; i < 
pflash_regions_count(pfl); ++i) {
hw/block/pflash_cfi02.c:889:    for (int i = 0; i < nb_regions; ++i) {
hw/i386/fw_cfg.c:39:    for (size_t i = 0; i < 
ARRAY_SIZE(fw_cfg_arch_wellknown_keys); i++) {
hw/i386/pc.c:1478:    for (size_t i = 0; i < ISA_NUM_IRQS; i++) {
hw/isa/lpc_ich9.c:70:    for (intx = 0; intx < PCI_NUM_PINS; intx++) {
hw/isa/lpc_ich9.c:126:        for (intx = 0; intx < PCI_NUM_PINS; intx++) {
hw/microblaze/xlnx-zynqmp-pmu.c:71:    for (int i = 0; i < 
XLNX_ZYNQMP_PMU_NUM_IPIS; i++) {
hw/microblaze/xlnx-zynqmp-pmu.c:124:    for (int i = 0; i < 
XLNX_ZYNQMP_PMU_NUM_IPIS; i++) {
hw/mips/cisco_c3600.c:472:    for (int i = 0; i < ISA_NUM_IRQS; i++) {
hw/mips/mips_malta.c:1471:    for (int i = 0; i < ISA_NUM_IRQS; i++) {
hw/ppc/fw_cfg.c:39:    for (size_t i = 0; i < 
ARRAY_SIZE(fw_cfg_arch_wellknown_keys); i++) {
hw/riscv/sifive_e.c:187:    for (int i = 0; i < 32; i++) {
hw/riscv/sifive_gpio.c:32:    for (int i = 0; i < SIFIVE_GPIO_PINS; i++) {
hw/riscv/sifive_gpio.c:360:    for (int i = 0; i < SIFIVE_GPIO_PINS; i++) {
hw/sd/aspeed_sdhci.c:130:    for (int i = 0; i < ASPEED_SDHCI_NUM_SLOTS; 
++i) {
hw/sparc/sun4m.c:117:    for (size_t i = 0; i < 
ARRAY_SIZE(fw_cfg_arch_wellknown_keys); i++) {
hw/sparc64/sun4u.c:108:    for (size_t i = 0; i < 
ARRAY_SIZE(fw_cfg_arch_wellknown_keys); i++) {
hw/usb/hcd-xhci.c:3553:    for (intr = 0; intr < xhci->numintrs; intr++) {
hw/virtio/vhost.c:454:        for (int i = 0; i < n_old_sections; i++) {
qemu-nbd.c:302:            for (size_t bit = 0; bit < 
ARRAY_SIZE(flag_names); bit++) {
target/i386/hvf/hvf.c:497:    for (int i = 0; i < 8; i++) {
target/i386/hvf/x86_decode.c:34:    for (int i = 0; i < 
decode->opcode_len; i++) {
tests/pflash-cfi02-test.c:343:    for (int region = 0; region < 
nb_erase_regions; ++region) {
tests/pflash-cfi02-test.c:407:    for (int region = 0; region < 
nb_erase_regions; ++region) {
tests/pflash-cfi02-test.c:447:    for (int region = 0; region < 
nb_erase_regions; ++region) {
tests/pflash-cfi02-test.c:448:        for (int i = 0; i < 
config->nb_blocs[region]; ++i) {
tests/pflash-cfi02-test.c:458:    for (int region = 0; region < 
nb_erase_regions; ++region) {
tests/pflash-cfi02-test.c:469:    for (int region = 0; region < 
nb_erase_regions; ++region) {
tests/pflash-cfi02-test.c:470:        for (int i = 0; i < 
config->nb_blocs[region]; ++i) {
tests/pflash-cfi02-test.c:658:    for (size_t i = 0; i < 
nb_configurations; ++i) {

[I introduced most of them without respecting the CODING_STYLE,
  but checkpatch didn't complained].

>> See the 'Declarations' section of CODING_STYLE.rst.
> 
> OK, that's a point. But since this gnu99 is a rather new option that we
> just introduced less than a year ago, we should maybe think of whether
> we want to allow this for for-loops now, too (since IMHO it's quite a
> nice feature in gnu99).

I agree with Thomas. I started to clean some signed/unsigned warnings 
and various cases the scope of some variables is confuse.

The related question is, is it OK to use size_t to iterate over an array?

   for (size_t i = 0; i < ARRAY_SIZE(array); i++) {
     ...
   }

Asking in this thread so we can modify CODING_STYLE accordingly :)

Re: [PATCH for-4.2] hw/i386: Fix compiler warning when CONFIG_IDE_ISA is disabled

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

[...]
> The related question is, is it OK to use size_t to iterate over an array?
>
>   for (size_t i = 0; i < ARRAY_SIZE(array); i++) {
>     ...
>   }

My rule of thumb on integer types is "whatever lets me avoid
not-obviously-safe conversions (implicit ones in particular) with the
least type cast clutter.

Quite often, int fits the bill.  But not always.

To reply to your example: depends on what's hiding in the ... :)

> Asking in this thread so we can modify CODING_STYLE accordingly :)

Re: [PATCH for-4.2] hw/i386: Fix compiler warning when CONFIG_IDE_ISA is disabled

[Thomas Huth]

On 18/11/2019 10.25, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
> [...]
>> The related question is, is it OK to use size_t to iterate over an array?
>>
>>   for (size_t i = 0; i < ARRAY_SIZE(array); i++) {
>>     ...
>>   }
> 
> My rule of thumb on integer types is "whatever lets me avoid
> not-obviously-safe conversions (implicit ones in particular) with the
> least type cast clutter.
> 
> Quite often, int fits the bill.  But not always.
> 
> To reply to your example: depends on what's hiding in the ... :)

The problem here is that ARRAY_SIZE() gives you an size_t, so the
compiler might complain about comparing signed int with unsigned size_t.

Thus if i is only used as array index in the "..." part, I think it's
fine to use size_t for i here.

 Thomas