Re: [Qemu-devel] [PATCH 8/9] lsi53c895a: avoid a warning from clang analyzer

[Blue Swirl]

On Sun, Sep 4, 2011 at 4:20 PM, Avi Kivity <address@hidden> wrote:
> On 09/04/2011 06:53 PM, Blue Swirl wrote:
>>
>> Avoid this warning from clang analyzer by adjusting the scope
>> of the variable:
>> /src/qemu/hw/lsi53c895a.c:895:5: warning: Value stored to 'id' is never
>> read
>>     id = (current_tag>>  8)&  0xf;
>>
>> Signed-off-by: Blue Swirl<address@hidden>
>> ---
>>  hw/lsi53c895a.c |   14 ++++++++------
>>  1 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
>> index 1643a63..9d900d0 100644
>> --- a/hw/lsi53c895a.c
>> +++ b/hw/lsi53c895a.c
>> @@ -883,7 +883,6 @@ static void lsi_do_msgout(LSIState *s)
>>      int len;
>>      uint32_t current_tag;
>>      lsi_request *current_req, *p, *p_next;
>> -    int id;
>>
>>      if (s->current) {
>>          current_tag = s->current->tag;
>> @@ -892,7 +891,6 @@ static void lsi_do_msgout(LSIState *s)
>>          current_tag = s->select_tag;
>>          current_req = lsi_find_by_tag(s, current_tag);
>>      }
>> -    id = (current_tag>>  8)&  0xf;
>>
>>      DPRINTF("MSG out len=%d\n", s->dbc);
>>      while (s->dbc) {
>> @@ -977,10 +975,14 @@ static void lsi_do_msgout(LSIState *s)
>>                 device, but this is currently not implemented (and seems
>> not
>>                 to be really necessary). So let's simply clear all queued
>>                 commands for the current device: */
>> -            id = current_tag&  0x0000ff00;
>> -            QTAILQ_FOREACH_SAFE(p,&s->queue, next, p_next) {
>> -                if ((p->tag&  0x0000ff00) == id) {
>> -                    scsi_req_cancel(p->req);
>> +            {
>> +                int id;
>> +
>> +                id = current_tag&  0x0000ff00;
>> +                QTAILQ_FOREACH_SAFE(p,&s->queue, next, p_next) {
>> +                    if ((p->tag&  0x0000ff00) == id) {
>> +                        scsi_req_cancel(p->req);
>> +                    }
>>                  }
>>              }
>>
>
> Why not keep id declared in the outer scope?  This extra indentation is
> annoying.

Also the variable could be dropped altogether simply with:
                if ((p->tag & 0x0000ff00) == (current_tag & 0x0000ff00)) {

I have no preference.