From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21093 invoked by alias); 11 Jan 2010 04:05:12 -0000 Received: (qmail 21082 invoked by uid 22791); 11 Jan 2010 04:05:11 -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; Mon, 11 Jan 2010 04:05:07 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id CC9F42BAB4D; Sun, 10 Jan 2010 23:05:05 -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 IRRQ-r0D-eqa; Sun, 10 Jan 2010 23:05:05 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 35DC92BAB4A; Sun, 10 Jan 2010 23:05:05 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id F3D92F595E; Mon, 11 Jan 2010 08:04:52 +0400 (RET) Date: Mon, 11 Jan 2010 04:05:00 -0000 From: Joel Brobecker To: Hui Zhu Cc: Michael Snyder , gdb-patches ml Subject: Re: [RFA/i386] Prec x86 MMX 3DNow! SSE SSE2 SSE3 SSSE3 SSE4 support Message-ID: <20100111040452.GA30569@adacore.com> References: <4B215151.6040608@vmware.com> <4B26825B.7000209@vmware.com> <4B2BDAEC.7090207@vmware.com> <20100109122833.GB2007@adacore.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: 2010-01/txt/msg00240.txt.bz2 > I am not sure it better can be the part of this patch. It doesn't > have relationship with sse insn support, right? This is why I said that this could be done in a followup patch. > Looks you have a lot of idea on it. I suggest you can work on it. :) Sorry, I have no interest at this point in time in working on the process record support. I am, however, spending a very large amount of time trying to review your code, provide comments, and answering your emails. The suggestion about fixing the way you handle errors, in particular the use of labels, is an important one, so please do not discard it on the basis that someone else can work on it. > + switch (tmpu8) > > Im not sure it clear or not. I think add more vals will make the code > not clear. I think you are wrong, but we can only know by comparing the two alternatives. Your code is using switch statements inside switch statements, and this allows this type of practice. But if it was me, I would have asked for each case block that requires non-trivial processing to be moved to a separate function. With that in mind, you'd have had to declare a new variable instead of reusing a generic one which has a meaningless name. In the present case, you can certainly declare a variable local to where you need it. For instance: case bla: { int regno; target_read_memory (addr, ®no); [...] This code looks a lot clearer to me than: case bla: target_read_memory (addr, &tmpu); That being said, this is not the most important part in the list of things I'd like to see being improved in the process record code. So, in the grand scheme of things, I'm fine if you are still not convinced. -- Joel