qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Thomas Huth <thuth@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>
Cc: QEMU Trivial <qemu-trivial@nongnu.org>,
	Eduardo Habkost <ehabkost@redhat.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [PATCH for-4.2] hw/i386: Fix compiler warning when CONFIG_IDE_ISA is disabled
Date: Fri, 15 Nov 2019 21:33:53 +0100	[thread overview]
Message-ID: <a2795653-8726-6bce-7183-2da59d70808e@redhat.com> (raw)
In-Reply-To: <be453a76-d5fe-62c3-6954-447be7fe646a@redhat.com>

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>



  reply	other threads:[~2019-11-15 20:35 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-15 14:50 [PATCH for-4.2] hw/i386: Fix compiler warning when CONFIG_IDE_ISA is disabled Thomas Huth
2019-11-15 15:34 ` Michael S. Tsirkin
2019-11-15 15:54 ` Peter Maydell
2019-11-15 15:54   ` Thomas Huth
2019-11-15 16:13     ` Paolo Bonzini
2019-11-15 16:13       ` Thomas Huth
2019-11-15 20:33         ` Philippe Mathieu-Daudé [this message]
2019-11-15 16:15     ` Peter Maydell
2019-11-15 16:12       ` Thomas Huth
2019-11-15 20:31         ` Philippe Mathieu-Daudé
2019-11-18  9:25           ` Markus Armbruster
2019-11-18  9:28             ` Thomas Huth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a2795653-8726-6bce-7183-2da59d70808e@redhat.com \
    --to=philmd@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).