* [Qemu-trivial] [PATCH] Add 'fall through' comments to case statements without break
@ 2012-01-09 17:29 ` Stefan Weil
0 siblings, 0 replies; 14+ messages in thread
From: Stefan Weil @ 2012-01-09 17:29 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, Stefan Weil
These comments are used by static code analysis tools and in code reviews
to avoid false warnings because of missing break statements.
The case statements handled here were reported by coverity.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
hw/pcnet.c | 1 +
json-lexer.c | 1 +
qemu-option.c | 4 ++++
3 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/hw/pcnet.c b/hw/pcnet.c
index cba253b..306dc6e 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -1505,6 +1505,7 @@ static void pcnet_bcr_writew(PCNetState *s, uint32_t rap, uint32_t val)
#ifdef PCNET_DEBUG
printf("BCR_SWS=0x%04x\n", val);
#endif
+ /* fall through */
case BCR_LNKST:
case BCR_LED1:
case BCR_LED2:
diff --git a/json-lexer.c b/json-lexer.c
index c21338f..3cd3285 100644
--- a/json-lexer.c
+++ b/json-lexer.c
@@ -301,6 +301,7 @@ static int json_lexer_feed_char(JSONLexer *lexer, char ch, bool flush)
case JSON_KEYWORD:
case JSON_STRING:
lexer->emit(lexer, lexer->token, new_state, lexer->x, lexer->y);
+ /* fall through */
case JSON_SKIP:
QDECREF(lexer->token);
lexer->token = qstring_new();
diff --git a/qemu-option.c b/qemu-option.c
index 6b23c31..a303f87 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -214,13 +214,17 @@ static int parse_option_size(const char *name, const char *value, uint64_t *ret)
switch (*postfix) {
case 'T':
sizef *= 1024;
+ /* fall through */
case 'G':
sizef *= 1024;
+ /* fall through */
case 'M':
sizef *= 1024;
+ /* fall through */
case 'K':
case 'k':
sizef *= 1024;
+ /* fall through */
case 'b':
case '\0':
*ret = (uint64_t) sizef;
--
1.7.2.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH] Add 'fall through' comments to case statements without break
@ 2012-01-09 17:29 ` Stefan Weil
0 siblings, 0 replies; 14+ messages in thread
From: Stefan Weil @ 2012-01-09 17:29 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, Stefan Weil
These comments are used by static code analysis tools and in code reviews
to avoid false warnings because of missing break statements.
The case statements handled here were reported by coverity.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
hw/pcnet.c | 1 +
json-lexer.c | 1 +
qemu-option.c | 4 ++++
3 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/hw/pcnet.c b/hw/pcnet.c
index cba253b..306dc6e 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -1505,6 +1505,7 @@ static void pcnet_bcr_writew(PCNetState *s, uint32_t rap, uint32_t val)
#ifdef PCNET_DEBUG
printf("BCR_SWS=0x%04x\n", val);
#endif
+ /* fall through */
case BCR_LNKST:
case BCR_LED1:
case BCR_LED2:
diff --git a/json-lexer.c b/json-lexer.c
index c21338f..3cd3285 100644
--- a/json-lexer.c
+++ b/json-lexer.c
@@ -301,6 +301,7 @@ static int json_lexer_feed_char(JSONLexer *lexer, char ch, bool flush)
case JSON_KEYWORD:
case JSON_STRING:
lexer->emit(lexer, lexer->token, new_state, lexer->x, lexer->y);
+ /* fall through */
case JSON_SKIP:
QDECREF(lexer->token);
lexer->token = qstring_new();
diff --git a/qemu-option.c b/qemu-option.c
index 6b23c31..a303f87 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -214,13 +214,17 @@ static int parse_option_size(const char *name, const char *value, uint64_t *ret)
switch (*postfix) {
case 'T':
sizef *= 1024;
+ /* fall through */
case 'G':
sizef *= 1024;
+ /* fall through */
case 'M':
sizef *= 1024;
+ /* fall through */
case 'K':
case 'k':
sizef *= 1024;
+ /* fall through */
case 'b':
case '\0':
*ret = (uint64_t) sizef;
--
1.7.2.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH] Add 'fall through' comments to case statements without break
2012-01-09 17:29 ` [Qemu-devel] " Stefan Weil
@ 2012-01-09 17:34 ` Peter Maydell
-1 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2012-01-09 17:34 UTC (permalink / raw)
To: Stefan Weil; +Cc: qemu-trivial, qemu-devel
On 9 January 2012 17:29, Stefan Weil <sw@weilnetz.de> wrote:
> These comments are used by static code analysis tools and in code reviews
> to avoid false warnings because of missing break statements.
>
> The case statements handled here were reported by coverity.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] Add 'fall through' comments to case statements without break
@ 2012-01-09 17:34 ` Peter Maydell
0 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2012-01-09 17:34 UTC (permalink / raw)
To: Stefan Weil; +Cc: qemu-trivial, qemu-devel
On 9 January 2012 17:29, Stefan Weil <sw@weilnetz.de> wrote:
> These comments are used by static code analysis tools and in code reviews
> to avoid false warnings because of missing break statements.
>
> The case statements handled here were reported by coverity.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-trivial] [PATCH] Add 'fall through' comments to case statements without break
2012-01-09 17:29 ` [Qemu-devel] " Stefan Weil
@ 2012-01-10 8:35 ` Stefan Hajnoczi
-1 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2012-01-10 8:35 UTC (permalink / raw)
To: Stefan Weil; +Cc: qemu-trivial, qemu-devel
On Mon, Jan 09, 2012 at 06:29:51PM +0100, Stefan Weil wrote:
> These comments are used by static code analysis tools and in code reviews
> to avoid false warnings because of missing break statements.
>
> The case statements handled here were reported by coverity.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> hw/pcnet.c | 1 +
> json-lexer.c | 1 +
> qemu-option.c | 4 ++++
> 3 files changed, 6 insertions(+), 0 deletions(-)
This reminds me of another questionable fall-through:
bt-host.c:bt_host_read():
while (s->len --)
switch (*pkt ++) {
...
case HCI_SCODATA_PKT:
if (s->len < 3)
goto bad_pkt;
pktlen = MIN(pkt[2] + 3, s->len);
s->len -= pktlen;
pkt += pktlen;
<--- fall-through or not?
default:
bad_pkt:
fprintf(stderr, "qemu: bad HCI packet type %02x\n", pkt[-1]);
}
It seems the code will skip HCI_SCODATA_PKT and report a warning (although type
pkt[-1] will be incorrect). Any thoughts?
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [Qemu-trivial] [PATCH] Add 'fall through' comments to case statements without break
@ 2012-01-10 8:35 ` Stefan Hajnoczi
0 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2012-01-10 8:35 UTC (permalink / raw)
To: Stefan Weil; +Cc: qemu-trivial, qemu-devel
On Mon, Jan 09, 2012 at 06:29:51PM +0100, Stefan Weil wrote:
> These comments are used by static code analysis tools and in code reviews
> to avoid false warnings because of missing break statements.
>
> The case statements handled here were reported by coverity.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> hw/pcnet.c | 1 +
> json-lexer.c | 1 +
> qemu-option.c | 4 ++++
> 3 files changed, 6 insertions(+), 0 deletions(-)
This reminds me of another questionable fall-through:
bt-host.c:bt_host_read():
while (s->len --)
switch (*pkt ++) {
...
case HCI_SCODATA_PKT:
if (s->len < 3)
goto bad_pkt;
pktlen = MIN(pkt[2] + 3, s->len);
s->len -= pktlen;
pkt += pktlen;
<--- fall-through or not?
default:
bad_pkt:
fprintf(stderr, "qemu: bad HCI packet type %02x\n", pkt[-1]);
}
It seems the code will skip HCI_SCODATA_PKT and report a warning (although type
pkt[-1] will be incorrect). Any thoughts?
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-trivial] [PATCH] Add 'fall through' comments to case statements without break
2012-01-10 8:35 ` [Qemu-devel] " Stefan Hajnoczi
@ 2012-01-10 21:52 ` Stefan Weil
-1 siblings, 0 replies; 14+ messages in thread
From: Stefan Weil @ 2012-01-10 21:52 UTC (permalink / raw)
To: Andrzej Zaborowski; +Cc: qemu-trivial, qemu-devel
Am 10.01.2012 09:35, schrieb Stefan Hajnoczi:
> This reminds me of another questionable fall-through:
>
> bt-host.c:bt_host_read():
>
> while (s->len --)
> switch (*pkt ++) {
> ...
> case HCI_SCODATA_PKT:
> if (s->len < 3)
> goto bad_pkt;
>
> pktlen = MIN(pkt[2] + 3, s->len);
> s->len -= pktlen;
> pkt += pktlen;
> <--- fall-through or not?
> default:
> bad_pkt:
> fprintf(stderr, "qemu: bad HCI packet type %02x\n", pkt[-1]);
> }
>
> It seems the code will skip HCI_SCODATA_PKT and report a warning
> (although type
> pkt[-1] will be incorrect). Any thoughts?
>
> Stefan
>
Hi Andrzej,
I think there should be a break statement at the end of the
HCI_SCODATA_PKT case. Could you please check this?
Regards,
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [Qemu-trivial] [PATCH] Add 'fall through' comments to case statements without break
@ 2012-01-10 21:52 ` Stefan Weil
0 siblings, 0 replies; 14+ messages in thread
From: Stefan Weil @ 2012-01-10 21:52 UTC (permalink / raw)
To: Andrzej Zaborowski; +Cc: qemu-trivial, Stefan Hajnoczi, qemu-devel
Am 10.01.2012 09:35, schrieb Stefan Hajnoczi:
> This reminds me of another questionable fall-through:
>
> bt-host.c:bt_host_read():
>
> while (s->len --)
> switch (*pkt ++) {
> ...
> case HCI_SCODATA_PKT:
> if (s->len < 3)
> goto bad_pkt;
>
> pktlen = MIN(pkt[2] + 3, s->len);
> s->len -= pktlen;
> pkt += pktlen;
> <--- fall-through or not?
> default:
> bad_pkt:
> fprintf(stderr, "qemu: bad HCI packet type %02x\n", pkt[-1]);
> }
>
> It seems the code will skip HCI_SCODATA_PKT and report a warning
> (although type
> pkt[-1] will be incorrect). Any thoughts?
>
> Stefan
>
Hi Andrzej,
I think there should be a break statement at the end of the
HCI_SCODATA_PKT case. Could you please check this?
Regards,
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH] Add 'fall through' comments to case statements without break
2012-01-10 8:35 ` [Qemu-devel] " Stefan Hajnoczi
@ 2012-01-12 13:05 ` andrzej zaborowski
-1 siblings, 0 replies; 14+ messages in thread
From: andrzej zaborowski @ 2012-01-12 13:05 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-trivial, Stefan Weil, qemu-devel
On 10 January 2012 09:35, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Jan 09, 2012 at 06:29:51PM +0100, Stefan Weil wrote:
>> These comments are used by static code analysis tools and in code reviews
>> to avoid false warnings because of missing break statements.
>>
>> The case statements handled here were reported by coverity.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>> hw/pcnet.c | 1 +
>> json-lexer.c | 1 +
>> qemu-option.c | 4 ++++
>> 3 files changed, 6 insertions(+), 0 deletions(-)
>
> This reminds me of another questionable fall-through:
>
> bt-host.c:bt_host_read():
>
> while (s->len --)
> switch (*pkt ++) {
> ...
> case HCI_SCODATA_PKT:
> if (s->len < 3)
> goto bad_pkt;
>
> pktlen = MIN(pkt[2] + 3, s->len);
> s->len -= pktlen;
> pkt += pktlen;
> <--- fall-through or not?
> default:
> bad_pkt:
> fprintf(stderr, "qemu: bad HCI packet type %02x\n", pkt[-1]);
> }
>
> It seems the code will skip HCI_SCODATA_PKT and report a warning (although type
> pkt[-1] will be incorrect). Any thoughts?
Yes, definitely there's a break missing. Bluetooth SCO links are like
USB Isochronous, so not really tested because they're only used by
streaming devices.
Cheers
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [Qemu-trivial] [PATCH] Add 'fall through' comments to case statements without break
@ 2012-01-12 13:05 ` andrzej zaborowski
0 siblings, 0 replies; 14+ messages in thread
From: andrzej zaborowski @ 2012-01-12 13:05 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-trivial, Stefan Weil, qemu-devel
On 10 January 2012 09:35, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Jan 09, 2012 at 06:29:51PM +0100, Stefan Weil wrote:
>> These comments are used by static code analysis tools and in code reviews
>> to avoid false warnings because of missing break statements.
>>
>> The case statements handled here were reported by coverity.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>> hw/pcnet.c | 1 +
>> json-lexer.c | 1 +
>> qemu-option.c | 4 ++++
>> 3 files changed, 6 insertions(+), 0 deletions(-)
>
> This reminds me of another questionable fall-through:
>
> bt-host.c:bt_host_read():
>
> while (s->len --)
> switch (*pkt ++) {
> ...
> case HCI_SCODATA_PKT:
> if (s->len < 3)
> goto bad_pkt;
>
> pktlen = MIN(pkt[2] + 3, s->len);
> s->len -= pktlen;
> pkt += pktlen;
> <--- fall-through or not?
> default:
> bad_pkt:
> fprintf(stderr, "qemu: bad HCI packet type %02x\n", pkt[-1]);
> }
>
> It seems the code will skip HCI_SCODATA_PKT and report a warning (although type
> pkt[-1] will be incorrect). Any thoughts?
Yes, definitely there's a break missing. Bluetooth SCO links are like
USB Isochronous, so not really tested because they're only used by
streaming devices.
Cheers
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH] Add 'fall through' comments to case statements without break
2012-01-12 13:05 ` [Qemu-devel] [Qemu-trivial] " andrzej zaborowski
@ 2012-01-12 13:16 ` Stefan Hajnoczi
-1 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2012-01-12 13:16 UTC (permalink / raw)
To: andrzej zaborowski; +Cc: qemu-trivial, Stefan Weil, qemu-devel
On Thu, Jan 12, 2012 at 1:05 PM, andrzej zaborowski <balrogg@gmail.com> wrote:
> On 10 January 2012 09:35, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>> On Mon, Jan 09, 2012 at 06:29:51PM +0100, Stefan Weil wrote:
>>> These comments are used by static code analysis tools and in code reviews
>>> to avoid false warnings because of missing break statements.
>>>
>>> The case statements handled here were reported by coverity.
>>>
>>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>>> ---
>>> hw/pcnet.c | 1 +
>>> json-lexer.c | 1 +
>>> qemu-option.c | 4 ++++
>>> 3 files changed, 6 insertions(+), 0 deletions(-)
>>
>> This reminds me of another questionable fall-through:
>>
>> bt-host.c:bt_host_read():
>>
>> while (s->len --)
>> switch (*pkt ++) {
>> ...
>> case HCI_SCODATA_PKT:
>> if (s->len < 3)
>> goto bad_pkt;
>>
>> pktlen = MIN(pkt[2] + 3, s->len);
>> s->len -= pktlen;
>> pkt += pktlen;
>> <--- fall-through or not?
>> default:
>> bad_pkt:
>> fprintf(stderr, "qemu: bad HCI packet type %02x\n", pkt[-1]);
>> }
>>
>> It seems the code will skip HCI_SCODATA_PKT and report a warning (although type
>> pkt[-1] will be incorrect). Any thoughts?
>
> Yes, definitely there's a break missing. Bluetooth SCO links are like
> USB Isochronous, so not really tested because they're only used by
> streaming devices.
Thanks for explaining. We can address this in a separate patch, sorry
for hijacking this thread :).
Stefan Weil: Your patch looks good.
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [Qemu-trivial] [PATCH] Add 'fall through' comments to case statements without break
@ 2012-01-12 13:16 ` Stefan Hajnoczi
0 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2012-01-12 13:16 UTC (permalink / raw)
To: andrzej zaborowski; +Cc: qemu-trivial, Stefan Weil, qemu-devel
On Thu, Jan 12, 2012 at 1:05 PM, andrzej zaborowski <balrogg@gmail.com> wrote:
> On 10 January 2012 09:35, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>> On Mon, Jan 09, 2012 at 06:29:51PM +0100, Stefan Weil wrote:
>>> These comments are used by static code analysis tools and in code reviews
>>> to avoid false warnings because of missing break statements.
>>>
>>> The case statements handled here were reported by coverity.
>>>
>>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>>> ---
>>> hw/pcnet.c | 1 +
>>> json-lexer.c | 1 +
>>> qemu-option.c | 4 ++++
>>> 3 files changed, 6 insertions(+), 0 deletions(-)
>>
>> This reminds me of another questionable fall-through:
>>
>> bt-host.c:bt_host_read():
>>
>> while (s->len --)
>> switch (*pkt ++) {
>> ...
>> case HCI_SCODATA_PKT:
>> if (s->len < 3)
>> goto bad_pkt;
>>
>> pktlen = MIN(pkt[2] + 3, s->len);
>> s->len -= pktlen;
>> pkt += pktlen;
>> <--- fall-through or not?
>> default:
>> bad_pkt:
>> fprintf(stderr, "qemu: bad HCI packet type %02x\n", pkt[-1]);
>> }
>>
>> It seems the code will skip HCI_SCODATA_PKT and report a warning (although type
>> pkt[-1] will be incorrect). Any thoughts?
>
> Yes, definitely there's a break missing. Bluetooth SCO links are like
> USB Isochronous, so not really tested because they're only used by
> streaming devices.
Thanks for explaining. We can address this in a separate patch, sorry
for hijacking this thread :).
Stefan Weil: Your patch looks good.
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-trivial] [PATCH] Add 'fall through' comments to case statements without break
2012-01-09 17:29 ` [Qemu-devel] " Stefan Weil
@ 2012-01-12 13:21 ` Stefan Hajnoczi
-1 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2012-01-12 13:21 UTC (permalink / raw)
To: Stefan Weil; +Cc: qemu-trivial, qemu-devel
On Mon, Jan 09, 2012 at 06:29:51PM +0100, Stefan Weil wrote:
> These comments are used by static code analysis tools and in code reviews
> to avoid false warnings because of missing break statements.
>
> The case statements handled here were reported by coverity.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> hw/pcnet.c | 1 +
> json-lexer.c | 1 +
> qemu-option.c | 4 ++++
> 3 files changed, 6 insertions(+), 0 deletions(-)
Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-01-12 13:21 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-09 17:29 [Qemu-trivial] [PATCH] Add 'fall through' comments to case statements without break Stefan Weil
2012-01-09 17:29 ` [Qemu-devel] " Stefan Weil
2012-01-09 17:34 ` [Qemu-trivial] " Peter Maydell
2012-01-09 17:34 ` Peter Maydell
2012-01-10 8:35 ` [Qemu-trivial] " Stefan Hajnoczi
2012-01-10 8:35 ` [Qemu-devel] " Stefan Hajnoczi
2012-01-10 21:52 ` Stefan Weil
2012-01-10 21:52 ` [Qemu-devel] " Stefan Weil
2012-01-12 13:05 ` [Qemu-trivial] [Qemu-devel] " andrzej zaborowski
2012-01-12 13:05 ` [Qemu-devel] [Qemu-trivial] " andrzej zaborowski
2012-01-12 13:16 ` [Qemu-trivial] [Qemu-devel] " Stefan Hajnoczi
2012-01-12 13:16 ` [Qemu-devel] [Qemu-trivial] " Stefan Hajnoczi
2012-01-12 13:21 ` Stefan Hajnoczi
2012-01-12 13:21 ` [Qemu-devel] " Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.