This is the mail archive of the gdb@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: GDB to C++ issue: deletion


On Friday 01 August 2008 11:53:24 Eli Zaretskii wrote:
> > From: =?iso-8859-1?q?Andr=E9_P=F6nitz?= <apoenitz@trolltech.com>
> > Date: Fri, 1 Aug 2008 10:54:20 +0200
> > 
> > [1] Of course, every now and then a Guru level coder shows up, replaces
> > one central trouble maker with something very cool, more efficient,
> > even more hand-crafted, that happens to work well without being
> > understood by aynone else. Then the Guru leaves, and the code becomes
> > a "no go" area for mere mortals...
> 
> Which is why we should request that every such guru documents her
> code, both in the sources and in gdbint.texinfo, to the level where
> mere mortals can hack on it later.  Unfortunately, not all of them
> comply.

Part of the need to document code vanishes if the code is
straight to the point and not blurred by "maintanance operations".

Let me give an example. There is e.g (random pick):

    /* .... The new continuation will be added at
       the front.  */
    void
    add_intermediate_continuation (void (*continuation_hook)
                                   (struct continuation_arg *),
                                   struct continuation_arg *arg_list)
    {
      struct continuation *continuation_ptr;

      continuation_ptr =
        (struct continuation *) xmalloc (sizeof (struct continuation));
      continuation_ptr->continuation_hook = continuation_hook;
      continuation_ptr->arg_list = arg_list;
      continuation_ptr->next = intermediate_continuation;
      intermediate_continuation = continuation_ptr;
    }

That could in "C-with-classes" look like

    /* ... */ 
    void
    prepend_intermediate_continuation (void (*continuation_hook)
                                   (continuation_arg *),
                                   continuation_arg *arg_list)
    {
      intermediate_continuation.prepend (
         new continuation (continuation_hook, arg_list));
    }

It's clear from a glimpse here that new continuation is _prepended_
to the list. Spotting this information in the code is as easy as spotting
it in the comment of the original version,  and there's no chance that
code and comment diverge. I also saves me a few seconds to check that
the code in the original version does indeed what the comment
promises. [Of course I could trust the reviewer that they'd never
accept out-of-sync comment, but there's still a slight and _needless_
risk there...]

Also, there are around 460 non-whitespace characters to be typed,
to be review, to be read  in the original code. The replacement has 200.
Similar results for line count... And it's not using complex C++ features, 
and the produced object code will be pretty much the same.

Andrà 

PS:

> Still, I think that it is a misrepresentation to say that GDB code
> cannot be understood by "mere mortals".  Most problems, as far as I
> remember from questions posted here, are about the high-level
> architectural aspects, asked by those who want to add support for some
> novel or unusual platform.  People who need to fix a bug in existing
> code normally don't have much trouble understanding it.

So I do not even qualify as "mere mortal" anymore  :-}

I typically fail to find the root of my gdb problems let alone to fix it.
E.g. at the beginning of the week I was trying to find out why in some
situations gdb claims to get "too few argument to function"
(infrun.c, around line 540), always with nargs == 1 and 
TYPE_NFIELDS (ftype) == 2

If this happens once then it does so consistently for anything that
has double quote characters on the commandline, say  

   p  "xxxx"

or

   call foo(0,1,2,"xxxx")

Incidentally,

   call foo(0,1,2,0) 

would _not_ complain about the wrong number. I spend quite a
while on that but I am still clueless. Unfortunately the situation
is quite complex (some gdb frontend trying to debug itself) so 
I did not try to ask for help yet. But according to your statement
I should not have much trouble to understand it ;-)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]