From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10687 invoked by alias); 8 Mar 2011 04:33:14 -0000 Received: (qmail 10678 invoked by uid 22791); 8 Mar 2011 04:33:12 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 08 Mar 2011 04:33:08 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id AB08F2BAD4F; Mon, 7 Mar 2011 23:33:06 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id DfVOBtd6b+kL; Mon, 7 Mar 2011 23:33:06 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 29E1B2BAD4E; Mon, 7 Mar 2011 23:33:06 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 4509D1459AD; Tue, 8 Mar 2011 08:32:42 +0400 (RET) Date: Tue, 08 Mar 2011 04:54:00 -0000 From: Joel Brobecker To: Hui Zhu Cc: Michael Snyder , "gdb-patches@sourceware.org" Subject: Re: [RFA] i386-tdep.c, i386_process_record, document fall-through case. Message-ID: <20110308043242.GI30306@adacore.com> References: <4D6EC5ED.3070307@vmware.com> <4D7137F6.4020000@vmware.com> <4D7527EE.9020204@vmware.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-03/txt/msg00513.txt.bz2 > As my poor understanding of C language, break or not break are both OK > for this part. I'm going to be a little extremist, and I don't really mean what I am about to ask, but: If the author of the code does not understand the code, and no other maintainer is able to review associated patches, is it time to remove that code? Speaking about the patch itself, I had a look, and I think, from what I understand, that, YES, the fallthrough is intended. IMO, it would have been clearer to write the code as follow: case 0xc4: /* les Gv */ case 0xc5: /* lds Gv */ case 0x0fb2: /* lss Gv */ case 0x0fb4: /* lfs Gv */ case 0x0fb5: /* lgs Gv */ if ((opcode == 0xc4 || opcode == 0xc5) && ir.regmap[X86_RECORD_R8_REGNUM]) { ir.addr -= 1; goto no_support; } if (i386_record_modrm (&ir)) return -1; (thus, not requiring a fallthrough) So patch is approved. -- Joel