From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4008 invoked by alias); 4 Mar 2010 07:39:10 -0000 Received: (qmail 3995 invoked by uid 22791); 4 Mar 2010 07:39:09 -0000 X-SWARE-Spam-Status: No, hits=-2.4 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; Thu, 04 Mar 2010 07:39:06 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 866062BABAC; Thu, 4 Mar 2010 02:39:04 -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 Aihd0h9mADw7; Thu, 4 Mar 2010 02:39:04 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id E2F652BAB8D; Thu, 4 Mar 2010 02:39:03 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id A5F0FF5894; Thu, 4 Mar 2010 11:38:55 +0400 (RET) Date: Thu, 04 Mar 2010 07:39:00 -0000 From: Joel Brobecker To: Hui Zhu Cc: gdb-patches ml , Mark Kettenis Subject: Re: [RFA] i386-tdep.c: fix a bug in prec i386 code Message-ID: <20100304073855.GK2832@adacore.com> References: 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: 2010-03/txt/msg00161.txt.bz2 > 2010-03-04 Hui Zhu > > * i386-tdep.c (i386_process_record): Change "addr" to "tmpu64". OK. As an aside, your code needs a really good thorough cleanup. I warned you already about the use of variables with a meaningless name, and you'll make this sort of mistake again for as long as you keep using them. However, my main point is that the use of a giant switch statement makes your code very hard to read and review. I really suggest that you create a new file, precord-i386.c where you put your stuff there, and instead of inlining the code inside each case, you define small contained procedures for each instruction (or instruction group). It'll be easier to find the code that handles such and such instruction, easier to write a ChangeLog entry that tells us more about where the change was made, and it'll make the switch block actually possible to read. -- Joel