From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46591) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y0RZh-0002FP-Qn for qemu-devel@nongnu.org; Mon, 15 Dec 2014 04:04:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y0RZb-0003Wv-Lr for qemu-devel@nongnu.org; Mon, 15 Dec 2014 04:04:17 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44347) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y0RZb-0003Wn-E9 for qemu-devel@nongnu.org; Mon, 15 Dec 2014 04:04:11 -0500 From: Markus Armbruster References: <20141212132840.GC8409@stefanha-thinkpad.redhat.com> Date: Mon, 15 Dec 2014 10:04:02 +0100 In-Reply-To: <20141212132840.GC8409@stefanha-thinkpad.redhat.com> (Stefan Hajnoczi's message of "Fri, 12 Dec 2014 13:28:40 +0000") Message-ID: <87oar5ffnh.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 1/4] block: vhdx - remove redundant comments List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: kwolf@redhat.com, amulya.lokesha@emc.com, Jeff Cody , qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com Stefan Hajnoczi writes: > On Mon, Dec 08, 2014 at 01:07:42AM -0500, Jeff Cody wrote: >> Minor cleanup. >> >> Signed-off-by: Jeff Cody >> --- >> block/vhdx.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/block/vhdx.c b/block/vhdx.c >> index 12bfe75..f1e1e2e 100644 >> --- a/block/vhdx.c >> +++ b/block/vhdx.c >> @@ -1109,8 +1109,8 @@ static coroutine_fn int vhdx_co_readv(BlockDriverState *bs, int64_t sector_num, >> /* check the payload block state */ >> switch (s->bat[sinfo.bat_idx] & VHDX_BAT_STATE_BIT_MASK) { >> case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */ Please drop this one as well, it's 100% certified noise. >> - case PAYLOAD_BLOCK_UNDEFINED: /* fall through */ >> - case PAYLOAD_BLOCK_UNMAPPED: /* fall through */ >> + case PAYLOAD_BLOCK_UNDEFINED: >> + case PAYLOAD_BLOCK_UNMAPPED: >> case PAYLOAD_BLOCK_ZERO: >> /* return zero */ >> qemu_iovec_memset(&hd_qiov, 0, 0, sinfo.bytes_avail); >> @@ -1280,8 +1280,8 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num, >> >> /* fall through */ This is a keeper. >> case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */ This isn't. >> - case PAYLOAD_BLOCK_UNMAPPED: /* fall through */ >> - case PAYLOAD_BLOCK_UNDEFINED: /* fall through */ >> + case PAYLOAD_BLOCK_UNMAPPED: >> + case PAYLOAD_BLOCK_UNDEFINED: >> bat_prior_offset = sinfo.file_offset; >> ret = vhdx_allocate_block(bs, s, &sinfo.file_offset); >> if (ret < 0) { > > Fall through comments are used by some static checkers. Not sure if all > checkers are smart enough to propagate the comment from adjacent case > statements. I would have left the comments alone but am okay with > merging this. I have never found a compiler or static checker so fanciful it actually nags its users about the perfectly obvious and common pattern "several case labels without code in between". The problematic pattern is "control falls from one case's *code* through to the next case". That one needs a comment indeed.