Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Joseph S. Myers" <joseph@codesourcery.com>
To: Nick Clifton <nickc@redhat.com>
Cc: "Richard Earnshaw (home)"
	<richard.earnshaw@buzzard.freeserve.co.uk>,
	    "binutils@sourceware.org" <binutils@sourceware.org>,
	    "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,
	    "gcc@gcc.gnu.org" <gcc@gcc.gnu.org>,
	"aph@redhat.com" <aph@redhat.com>
Subject: Re: RFC: Collecting together binary file attributes into a single file.
Date: Thu, 29 Sep 2011 17:57:00 -0000	[thread overview]
Message-ID: <Pine.LNX.4.64.1109291524410.26375@digraph.polyomino.org.uk> (raw)
In-Reply-To: <4E847BFD.1000807@redhat.com>

On Thu, 29 Sep 2011, Nick Clifton wrote:

> Hi Richard,
> 
> > I don't think it's a good idea to have the attributes of
> > every CPU we support in a single file.  That's going to
> > get unmaintainable very quickly.
> 
> Really - why ?
> 
> These attributes are mostly static.  Some new ones might be added from time to
> time, but baring the introduction of new ports I do not see any other changes
> that re likely to happen.

Among other things, it's quite possible that two targets will have names 
(defined in their ABIs) that conflict.  And the patch appears to be 
duplicating the C6X definitions (also present in tic6x-attrs.h for use in 
places that want to build an array as well as those just defining an enum) 
which is hardly desirable.

If you want a shared header *for ARM EABI attributes*, defining a header 
restricted just to the required definitions (and making sure everything 
relevant uses it) seems reasonable.

Now, all the definitions get included at once in readelf.c, so any 
conflicts would be visible there, but really I don't think that's a good 
design.  Rather than all those hardcoded switch statements I think it 
would be better to have e.g. readelf-arm.c that defines the ARM-specific 
functions and data (with readelf.c looking up function pointers in a 
structure exported by each architecture's file and identified by the 
e_machine value rather than using a switch statement in each place).  You 
might then have just one array that contains pointers to all the target 
structures.

-- 
Joseph S. Myers
joseph@codesourcery.com


      reply	other threads:[~2011-09-29 15:33 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-29  9:38 Nick Clifton
2011-09-29 10:12 ` Richard Earnshaw (home)
2011-09-29 14:14   ` Nick Clifton
2011-09-29 17:57     ` Joseph S. Myers [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.64.1109291524410.26375@digraph.polyomino.org.uk \
    --to=joseph@codesourcery.com \
    --cc=aph@redhat.com \
    --cc=binutils@sourceware.org \
    --cc=gcc@gcc.gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=nickc@redhat.com \
    --cc=richard.earnshaw@buzzard.freeserve.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox