Re: symbolic debug of loadable modules with kgdb light

[Joel Brobecker <brobecker@adacore.com>]

Caz,

I have a few remarks:

> I have attached the patch against gdb-6.8.

The patches first need to be brought up to date with our CVS head.
Our gdb-6.8 branch is more or less dead at this point, and the
current CVS head is sometimes significantly different from gdb-6.8.

Also, the patches should be broken down into small patches. I don't
know how well the other maintainers know the Linux kernel (I suspect
some of them know it very well), but most of us don't have much time
available to do code review. So breaking down the patches and explaining
what they are about is very very very very very very helpful. If you
don't do that, unless someone from the GDB Maintainers group is
expecially motivated in getting the feature you are submitting,
you are dramatically reducing your chances of us reviewing your patch.

> - This patch is based on http://kgdb.cvs.sourceforge.net/viewvc/kgdb/gdb/. I
> removed garbage as mush as possible.

I tried to have a look at who the authors are. The changes need to be
assigned to the FSF. I'm not a big specialist in legal matters, but
I believe that only the author of the changes can do that; someone
else who knows better about might want to confirm or correct.  There
is a provision for changes that are obvious, or small (usually less
than 10 lines). But this patch does not qualify. Some parts of it
might.

> But it still has the code which I don't know what it is.

As far as I am concerned, this is a problem. I personally am not going
to approve changes whose purpose I don't understand. My approach in
this case has always been to leave the change aside, and wait until
I hit a problem. If that change ends up fixing the problem, at least
I will know why (and I will make sure to add a comment besides the
code).

> - I haven't run testsuite because I could not find how to do that while I
> run make in testsuite directory.

The magic command for testing your changes natively is "make check".
What you need to do is run the testsuite before and after your
changes, and make sure that it does not introduce any regression.
Somes parts of the patch will be hard to exercise except manually,
since they refer to debugging kernel modules, which I don't think
is something that our testsuite knows how to do...

I hope all the above doesn't discourage you...

-- 
Joel