Patchwork ide/atapi: fix set but unused

login
register
mail settings
Submitter Alon Levy
Date April 28, 2011, 8:07 a.m.
Message ID <1303978031-24366-1-git-send-email-alevy@redhat.com>
Download mbox | patch
Permalink /patch/93195/
State New
Headers show

Comments

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

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));