From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7332 invoked by alias); 25 Apr 2012 15:29:26 -0000 Received: (qmail 7320 invoked by uid 22791); 25 Apr 2012 15:29:24 -0000 X-SWARE-Spam-Status: No, hits=-3.5 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_HOSTKARMA_NO,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_BJ 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; Wed, 25 Apr 2012 15:29:09 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 590FD1C6E73; Wed, 25 Apr 2012 11:29:08 -0400 (EDT) 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 UvWBez5Dn1Zy; Wed, 25 Apr 2012 11:29:08 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 1E4FE1C6E71; Wed, 25 Apr 2012 11:29:07 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 5B77B145616; Wed, 25 Apr 2012 08:28:47 -0700 (PDT) Date: Wed, 25 Apr 2012 15:54:00 -0000 From: Joel Brobecker To: "Maciej W. Rozycki" Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] microMIPS support Message-ID: <20120425152847.GG10958@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: 2012-04/txt/msg00865.txt.bz2 I could only scan the patch briefly for today, but noticed something that caught my attention: > +/* For backwards compatibility we default to MIPS16. This flag is > + overridden as soon as unambiguous ELF file flags tell us the > + compressed ISA encoding used. */ > +static const char mips_compression_mips16[] = "mips16"; > +static const char mips_compression_micromips[] = "micromips"; > +static const char *mips_compression_strings[] = { > + mips_compression_mips16, > + mips_compression_micromips, > + NULL > +}; We usually provide this sort of feature a little differently: Instead of 2 values that are adjusted automatically by the debugger, we provide 3 values: auto, mips16, and micromips. If auto, then the debugger has to guess, possibly defaulting to mips16 if guessing did not work. But if the user sets the setting to either of the non-auto values, then the setting should be honored, even if the user is wrong. This is usually implemented using two variables: One representing the setting, and one representing the actual value. This brings me to the next question: Could this be an objfile-specific setting? In other words, is it possible to that the same executable might have one objfile using micromips while another might still be using mips16? (this might be the stupidest question of the week...). If not, I still believe that this is at least an inferior-specific property. With multi-inferior debugging being possible, I can see how one debugs two programs using a different setting. In that case, you need to store that information in the inferior private data (I do that in ada-tasks, IIRC, for storing the layout of some data structures). (oops, style issue as well, the opening curly bracket should be at the start of the next line - we've seen a lot of your style too, but I think it should be fixed) -- Joel