Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Jim Blandy <jimb@zwingli.cygnus.com>
To: Daniel Berlin <dan@cgsoftware.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: Rewriting the type system
Date: Tue, 12 Jun 2001 10:16:00 -0000	[thread overview]
Message-ID: <npofrty5wv.fsf@zwingli.cygnus.com> (raw)
In-Reply-To: <87d78aluw6.fsf@cgsoftware.com>

Daniel Berlin <dan@cgsoftware.com> writes:
> Jim, GDB development is moving a lot slower than it should.  If
> someone told me, after just rewriting the typesystem, that i needed to
> redo it from scratch, i'd probably just start making my own GDB
> releases instead (in effect, forking GDB).  Not out of spite or
> anything, just so i could get stuff done without having to keep track
> of tons of patches that go months without review.

I apologize if I have been slow to review patches from time to time.
I don't like to feel that I'm a bottleneck in GDB development.

However, in your case, Dan, I think we can back up and ask a more
fundamental question.  You're a smart guy; you've got a lot of energy;
you have a lot of time, too, apparently.  You've been working on GDB
for at least a year and a half now.  You've taken on large, ambitious
tasks.

So, given all that, why are you in the position of waiting for my
approval for your changes?  Why aren't you listed as a maintainer of
the Dwarf2 reader and the symtab reader?

It's because, for whatever reason, you don't take the time to make
your changes correct.

Here is part of the function gnuv3_rtti_type in gnu-v3-abi.c, from
your GCC V3 C++ ABI support patch:

    coreptr =
      *((CORE_ADDR *) (VALUE_CONTENTS_ALL (v) + VALUE_OFFSET (v) +
                       VALUE_EMBEDDED_OFFSET (v)));
    coreptr -= 12;
    *top =
      value_as_long (value_at
                     (builtin_type_int, coreptr, VALUE_BFD_SECTION (v)));
    coreptr += 8;
    vp = value_at (builtin_type_int, coreptr, VALUE_BFD_SECTION (v));

Here we have, in the space of less than a dozen lines of code:

- host == target assumptions (why are you applying `*' to
  target-format data?)

- sizeof (foo) assumptions (what is 8?  what is 12?)

It makes me angry to be asked to review this kind of patch.  If you're
not going to bother with even the most basic rules of GDB development,
why should I bother to read your patches?

You have an extensive history of reverted changes:

2000-11-07  Daniel Berlin  <dberlin@redhat.com>

	* dwarf2read.c: Revert June 5th change for caching of types,
	as per Jim Blandy's request.

and the changes to lookup_symbol that were broken, the minsym sorting
issue that Peter Schauer called you on, etc.

One of the reasons it took me three or four months to review your GCC
V3 C++ ABI changes was that I had to learn enough C++ to be able to
understand the ABI spec.  Isn't that odd?  Why do we need to have our
C++ maintainer's patches reviewed by someone who doesn't know the
language?  As someone said recently, "This, IMHO, is amazingly silly."

Given your record, I'm not sure I think you should be the GDB C++
maintainer, or the maintainer of any part of GDB.


  parent reply	other threads:[~2001-06-12 10:16 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-06-07 23:22 obvious set_cu_language patch Per Bothner
2001-06-07 23:50 ` Daniel Berlin
2001-06-08 11:01   ` Per Bothner
2001-06-08 14:04     ` Stan Shebs
2001-06-08 14:34       ` Daniel Berlin
2001-06-08 13:04 ` Andrew Cagney
     [not found]   ` <m2pucevgf6.fsf@kelso.bothner.com>
2001-06-08 13:51     ` Daniel Berlin
     [not found]       ` <8766e6eke4.fsf@creche.redhat.com>
2001-06-08 14:23         ` Per Bothner
     [not found]       ` <npelsq28sh.fsf_-_@zwingli.cygnus.com>
2001-06-11 11:43         ` Rewriting the type system Daniel Berlin
2001-06-11 16:58           ` Stan Shebs
2001-06-12  1:44           ` Eli Zaretskii
2001-06-12  9:12             ` Daniel Berlin
2001-06-12 10:01               ` Eli Zaretskii
2001-06-12 10:16           ` Jim Blandy [this message]
2001-06-12 10:44             ` Daniel Berlin
2001-06-12 14:02               ` Stan Shebs
2001-06-13  1:45                 ` Eli Zaretskii
2001-06-12 11:08             ` Daniel Berlin
2001-06-12 14:03           ` Andrew Cagney
2001-06-12 21:37             ` Daniel Berlin
2001-06-25 14:13 ` obvious set_cu_language patch Elena Zannoni

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=npofrty5wv.fsf@zwingli.cygnus.com \
    --to=jimb@zwingli.cygnus.com \
    --cc=dan@cgsoftware.com \
    --cc=gdb-patches@sources.redhat.com \
    /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