From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:43978) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RlKLX-0004IL-L7 for qemu-devel@nongnu.org; Thu, 12 Jan 2012 08:05:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RlKLT-00051S-1V for qemu-devel@nongnu.org; Thu, 12 Jan 2012 08:05:35 -0500 MIME-Version: 1.0 In-Reply-To: <20120110083555.GA13145@stefanha-thinkpad.localdomain> References: <1326130191-20344-1-git-send-email-sw@weilnetz.de> <20120110083555.GA13145@stefanha-thinkpad.localdomain> Date: Thu, 12 Jan 2012 14:05:28 +0100 Message-ID: From: andrzej zaborowski Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH] Add 'fall through' comments to case statements without break List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: qemu-trivial@nongnu.org, Stefan Weil , qemu-devel@nongnu.org On 10 January 2012 09:35, Stefan Hajnoczi 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 review= s >> to avoid false warnings because of missing break statements. >> >> The case statements handled here were reported by coverity. >> >> Signed-off-by: Stefan Weil >> --- >> =C2=A0hw/pcnet.c =C2=A0 =C2=A0| =C2=A0 =C2=A01 + >> =C2=A0json-lexer.c =C2=A0| =C2=A0 =C2=A01 + >> =C2=A0qemu-option.c | =C2=A0 =C2=A04 ++++ >> =C2=A03 files changed, 6 insertions(+), 0 deletions(-) > > This reminds me of another questionable fall-through: > > bt-host.c:bt_host_read(): > > while (s->len --) > =C2=A0 =C2=A0switch (*pkt ++) { > =C2=A0 =C2=A0... > =C2=A0 =C2=A0case HCI_SCODATA_PKT: > =C2=A0 =C2=A0 =C2=A0 =C2=A0if (s->len < 3) > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0goto bad_pkt; > > =C2=A0 =C2=A0 =C2=A0 =C2=A0pktlen =3D MIN(pkt[2] + 3, s->len); > =C2=A0 =C2=A0 =C2=A0 =C2=A0s->len -=3D pktlen; > =C2=A0 =C2=A0 =C2=A0 =C2=A0pkt +=3D pktlen; > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 <--- fall-through or not? > =C2=A0 =C2=A0default: > =C2=A0 =C2=A0bad_pkt: > =C2=A0 =C2=A0 =C2=A0 =C2=A0fprintf(stderr, "qemu: bad HCI packet type %02= x\n", pkt[-1]); > =C2=A0 =C2=A0} > > It seems the code will skip HCI_SCODATA_PKT and report a warning (althoug= h type > pkt[-1] will be incorrect). =C2=A0Any 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