From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17846 invoked by alias); 9 Jan 2009 18:17:45 -0000 Received: (qmail 17837 invoked by uid 22791); 9 Jan 2009 18:17:44 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 09 Jan 2009 18:17:29 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id D40032A9681; Fri, 9 Jan 2009 13:17:27 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id ngBX8CiMkUNT; Fri, 9 Jan 2009 13:17:27 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 080B22A9653; Fri, 9 Jan 2009 13:17:27 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 15B57E7ACD; Fri, 9 Jan 2009 22:17:20 +0400 (RET) Date: Fri, 09 Jan 2009 18:17:00 -0000 From: Joel Brobecker To: Caz Yokoyama Cc: gdb@sourceware.org Subject: Re: symbolic debug of loadable modules with kgdb light Message-ID: <20090109181720.GD31296@adacore.com> References: <20090109041826.GD20227@adacore.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.2i Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org X-SW-Source: 2009-01/txt/msg00047.txt.bz2 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