This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFA]: Add misc_obstack to object files.
- To: dan at cgsoftware dot com, ezannoni at cygnus dot com
- Subject: Re: [RFA]: Add misc_obstack to object files.
- From: Michael Elizabeth Chastain <chastain at cygnus dot com>
- Date: Mon, 25 Jun 2001 22:27:06 -0700
- Cc: gdb-patches at sources dot redhat dot com
I'd like to throw my hat in the ring, too. I spent a whole weekend
several months ago changing the type of all the section offset tables.
I wound up with a 3000 line patch that I didn't even try to get
approved. (I was working on PR gdb/29, "gdb has fixed size MAX_SECTIONS").
So I care about section offset tables.
> I'm just adding a bin for colored paper, so people will put that stuff
> in there instead in the first place, and it will be plainly clear that
> they are supposed to be doing so.
So you don't have a manifestation of a bug in your hand; rather, you are
cleaning up something that is broken. That's fine. I am comfortable
with your rationale, now that I know what it is.
I see three specific problems:
. in lines 1984-1993 of symfile.c, you change the "psymtab" pointer from
psymbol_obstack to misc_obstack. It's not obvious why this structure
is on misc_obstack. Since it's not obvious, a comment would help.
. as Elena mentioned, there are other files which allocate section
offsets on the psymbol_obstack. I see somread.c in particular.
Right now it's difficult to test somread.c, because even if you have
access to hppa hardware, FSF gdb does not link for that configuration
(PR gdb/63). If you can't test it, and you don't want to go for a
blind patch to somread.c, then I think that a FIXME comment would be
appropriate, to explain to the next hpux maintainer why this target
allocates section_offsets on psymbol_objstack when other targets use
misc_obstack. Something like: "FIXME: this should be misc_obstack,
same as symfile.c, but I can't test this file so I'm leaving it as
psymbol_obstack, which ought to continue working for now".
. the new comment on lines 121-137 of objfiles.c is not true yet,
even after applying your patch. For example, dbxread.c puts
DBX_STRINGTAB on psymbol_obstack. So I challenge your claim
"All the stuff in objfile that was on the psymbol obstack,
but didnt' belong, is in the misc obstack ...".
> The only thing even related to psymtabs there is the fact that they
> psymtab structure itself is allocated on the misc obstack, since it
> isn't a partial symbol either. It's a partial symbol table structure,
> containing pointers to partial symbols.
If you put this in a comment, that would answer my first question
(which might be the same as Elena's question about an extraneous patch
fragment, but I don't know what fragment Elena is referring to).
> I could do them all now if it helps, but it'll make the patch bigger.
I like it better this way, one structure at a time. I think that
Elena's looking for some sense of whether this is a standalone patch
or part of a bigger series -- not one big patch.
Here are the reasons that I prefer series:
. It's easier to proof-read one concept at a time.
. It's easier to read the first "proof of concept" when there's only
one structure in it, rather than old structures.
. If something breaks in the future, and I need to binary-search old
versions of gdb, this improves the granularity that I can resolve the
bug too. This is important for symbol table patches as a bug can
manifest subtly (e.g. lookup_symbol bug -> infinite loop in "maint"
command when running on an inferior copy of gdb).
Basically there are two different ideas here. First there is the idea of
what should actually be in gdb. I'm comfortable with your vision there.
Second there is the idea of structuring one's work so that other people
can deduce that it actually implements the concept (including checking
for implementation bugs).
Just my two cents. I'll try not to write again, as Elena is the
maintainer here.
Michael