Re: Rewriting the type system

Daniel Berlin
2001-06-12T11:08:00Z

Jim Blandy <jimb@zwingli.cygnus.com> writes:

I'm also curious Jim, did it make you "angry" to review Jason
Merril's 3 line fix for dbxread.c and dwarfread.c, that he submitted
12-15-2000?
You know, that completely incorrect code that said "If we are
processing gcc compilation, it must be old abi demangling"?
Oh wait, you never reviewed it.
That's right.
Elena did, over a month (1 month, 1 day to be exact) later.

Why is it I can get patches that fix crashing bugs in "set
demangle-style" reviewed in hours, but when i, or others, submit
something in your area, it's like a black hole?

I'm pretty sure you don't want me to start dredging out all the
examples where either you've been plain old non-responsive, or taken
months.
Or code that makes me angry to look at, that you've written.
For fun, let's take our recent friend, find_toplevel_char.

2000-01-27 Jim Blandy <jimb@cygnus.com>

* symtab.c (decode_line_1): Don't let commas that are within
quotes or parenthesis terminate the line spec. Don't use pp when
removing the final double quote of a double-quoted string. Don't
forget to skip the opening double quote. I have no clue whether
this change is correct; probably we've just moved this function
from one buggy place to another buggy place, and never came within
an outhouse whiff of correctness.
(find_toplevel_char): New function.

Funny, doesn't seem to me like you tried to make sure the code was
correct.

In fact, it was your addition that caused the problem.

Like I said, you *really* don't want to start playing that
"correctness" of code game.
Neither of us will win. in fact, nobody would win.
The changelogs show that, for instance, Andrew had his last patch to
dwarf2read.c reverted. 3 years later, of course.

I've had *one* patch reverted.
The changelogs clearly show this.
We've all had about the same number of patches reverted.
So please, stop trying to pretend i'm a special case here, or that
i'm a bad egg, who submits bad patches.
It's simply not true.

--Dan

> 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.

--
"I was once arrested for walking in someone else's sleep.
"-Steven Wright