qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [Qemu-devel] [PATCH] Add 'fall through' comments to c


From: Stefan Hajnoczi
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH] Add 'fall through' comments to case statements without break
Date: Thu, 12 Jan 2012 13:16:17 +0000

On Thu, Jan 12, 2012 at 1:05 PM, andrzej zaborowski <address@hidden> wrote:
> On 10 January 2012 09:35, Stefan Hajnoczi <address@hidden> 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 <address@hidden>
>>> ---
>>>  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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]