diff mbox

ide/atapi: fix set but unused

Message ID 1303978031-24366-1-git-send-email-alevy@redhat.com
State New
Headers show

Commit Message

Alon Levy April 28, 2011, 8:07 a.m. UTC 
Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/ide/atapi.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

Comments

Paolo Bonzini April 28, 2011, 12:07 p.m. UTC | #1 
On 04/28/2011 10:07 AM, Alon Levy wrote:
> Signed-off-by: Alon Levy<alevy@redhat.com>
> ---
>   hw/ide/atapi.c |    4 +++-
>   1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index 690a0ab..81fa01b 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -1080,12 +1080,14 @@ static const struct {
>
>   void ide_atapi_cmd(IDEState *s)
>   {
> +#ifdef DEBUG_IDE_ATAPI
>       const uint8_t *packet;
> +#endif
>       uint8_t *buf;
>
> -    packet = s->io_buffer;
>       buf = s->io_buffer;
>   #ifdef DEBUG_IDE_ATAPI
> +    packet = s->io_buffer;
>       {
>           int i;
>           printf("ATAPI limit=0x%x packet:", s->lcyl | (s->hcyl<<  8));

ACK

... but why don't we allow interspersing declarations and statements? 
There are already so many C99 and GCC extensions in use in QEMU.

Paolo
Stefan Weil April 28, 2011, 12:16 p.m. UTC | #2 
Am 28.04.2011 14:07, schrieb Paolo Bonzini:
> On 04/28/2011 10:07 AM, Alon Levy wrote:
>> Signed-off-by: Alon Levy<alevy@redhat.com>
>> ---
>>   hw/ide/atapi.c |    4 +++-
>>   1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
>> index 690a0ab..81fa01b 100644
>> --- a/hw/ide/atapi.c
>> +++ b/hw/ide/atapi.c
>> @@ -1080,12 +1080,14 @@ static const struct {
>>
>>   void ide_atapi_cmd(IDEState *s)
>>   {
>> +#ifdef DEBUG_IDE_ATAPI
>>       const uint8_t *packet;
>> +#endif
>>       uint8_t *buf;
>>
>> -    packet = s->io_buffer;
>>       buf = s->io_buffer;
>>   #ifdef DEBUG_IDE_ATAPI
>> +    packet = s->io_buffer;
>>       {
>>           int i;
>>           printf("ATAPI limit=0x%x packet:", s->lcyl | (s->hcyl<<  8));
>
> ACK
>
> ... but why don't we allow interspersing declarations and statements? 
> There are already so many C99 and GCC extensions in use in QEMU.
>
> Paolo

What about using buf instead of packet (no need for extensions, reduces 
code by 4 lines)?

Stefan W.
Paolo Bonzini April 28, 2011, 12:35 p.m. UTC | #3 
On 04/28/2011 02:16 PM, Stefan Weil wrote:
>
> What about using buf instead of packet (no need for extensions, reduces
> code by 4 lines)?

Uh, you're right!... I was assuming some side effect later, but after 
the refactoring this is indeed the only place where packet is used in 
the function, and it could use buf.

Paolo
diff mbox

Patch

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 690a0ab..81fa01b 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -1080,12 +1080,14 @@  static const struct {
 
 void ide_atapi_cmd(IDEState *s)
 {
+#ifdef DEBUG_IDE_ATAPI
     const uint8_t *packet;
+#endif
     uint8_t *buf;
 
-    packet = s->io_buffer;
     buf = s->io_buffer;
 #ifdef DEBUG_IDE_ATAPI
+    packet = s->io_buffer;
     {
         int i;
         printf("ATAPI limit=0x%x packet:", s->lcyl | (s->hcyl << 8));