* [Qemu-devel] [PATCH 8/9] lsi53c895a: avoid a warning from clang analyzer
@ 2011-09-04 15:53 Blue Swirl
2011-09-04 16:20 ` Avi Kivity
0 siblings, 1 reply; 5+ messages in thread
From: Blue Swirl @ 2011-09-04 15:53 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 1740 bytes --]
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 <blauwirbel@gmail.com>
---
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);
+ }
}
}
--
1.6.2.4
[-- Attachment #2: 0008-lsi53c895a-avoid-a-warning-from-clang-analyzer.patch --]
[-- Type: text/x-diff, Size: 2236 bytes --]
From f787e79fa55affb8999ffc1765ea486c8edd46ce Mon Sep 17 00:00:00 2001
Message-Id: <f787e79fa55affb8999ffc1765ea486c8edd46ce.1315150286.git.blauwirbel@gmail.com>
In-Reply-To: <70f99a25b7732d4c9ea54f74c089ccb9bb323ea6.1315150286.git.blauwirbel@gmail.com>
References: <70f99a25b7732d4c9ea54f74c089ccb9bb323ea6.1315150286.git.blauwirbel@gmail.com>
From: Blue Swirl <blauwirbel@gmail.com>
Date: Sun, 4 Sep 2011 11:22:31 +0000
Subject: [PATCH 8/9] lsi53c895a: avoid a warning from clang analyzer
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 <blauwirbel@gmail.com>
---
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);
+ }
}
}
--
1.7.2.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] lsi53c895a: avoid a warning from clang analyzer
2011-09-04 15:53 [Qemu-devel] [PATCH 8/9] lsi53c895a: avoid a warning from clang analyzer Blue Swirl
@ 2011-09-04 16:20 ` Avi Kivity
2011-09-04 16:27 ` Blue Swirl
2011-09-04 17:05 ` Peter Maydell
0 siblings, 2 replies; 5+ messages in thread
From: Avi Kivity @ 2011-09-04 16:20 UTC (permalink / raw)
To: Blue Swirl; +Cc: Paolo Bonzini, qemu-devel
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<blauwirbel@gmail.com>
> ---
> 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.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] lsi53c895a: avoid a warning from clang analyzer
2011-09-04 16:20 ` Avi Kivity
@ 2011-09-04 16:27 ` Blue Swirl
2011-09-05 5:35 ` Avi Kivity
2011-09-04 17:05 ` Peter Maydell
1 sibling, 1 reply; 5+ messages in thread
From: Blue Swirl @ 2011-09-04 16:27 UTC (permalink / raw)
To: Avi Kivity; +Cc: Paolo Bonzini, qemu-devel
On Sun, Sep 4, 2011 at 4:20 PM, Avi Kivity <avi@redhat.com> 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<blauwirbel@gmail.com>
>> ---
>> 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] lsi53c895a: avoid a warning from clang analyzer
2011-09-04 16:20 ` Avi Kivity
2011-09-04 16:27 ` Blue Swirl
@ 2011-09-04 17:05 ` Peter Maydell
1 sibling, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2011-09-04 17:05 UTC (permalink / raw)
To: Avi Kivity; +Cc: Blue Swirl, Paolo Bonzini, qemu-devel
On 4 September 2011 17:20, Avi Kivity <avi@redhat.com> wrote:
> Why not keep id declared in the outer scope? This extra indentation is
> annoying.
Personally I find that in a 125 line long function, declaring
a variable at function scope when it's actually used only in
a very small section of the code makes for worse readability:
you have to search through to confirm that it really is only
used in that small section. It gets worse if you have one
variable reused for several unrelated things.
So I think either the patch as submitted or the other approach
Blue Swirl suggests of just dropping the variable are better
than leaving id at function scope.
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] lsi53c895a: avoid a warning from clang analyzer
2011-09-04 16:27 ` Blue Swirl
@ 2011-09-05 5:35 ` Avi Kivity
0 siblings, 0 replies; 5+ messages in thread
From: Avi Kivity @ 2011-09-05 5:35 UTC (permalink / raw)
To: Blue Swirl; +Cc: Paolo Bonzini, qemu-devel
On 09/04/2011 07:27 PM, Blue Swirl wrote:
> >> 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.
Yet another option is to allow intermixing declarations and statements
(which C99 allows), though it tends to get some traditionalists worked up.
It's particularly nice in for () loops.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-09-05 5:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-04 15:53 [Qemu-devel] [PATCH 8/9] lsi53c895a: avoid a warning from clang analyzer Blue Swirl
2011-09-04 16:20 ` Avi Kivity
2011-09-04 16:27 ` Blue Swirl
2011-09-05 5:35 ` Avi Kivity
2011-09-04 17:05 ` Peter Maydell
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).