From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16847 invoked by alias); 17 Mar 2009 15:57:53 -0000 Received: (qmail 16837 invoked by uid 22791); 17 Mar 2009 15:57:51 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_14,J_CHICKENPOX_93 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; Tue, 17 Mar 2009 15:57:46 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id CA8892BAC35; Tue, 17 Mar 2009 11:57:44 -0400 (EDT) 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 iI3L4vDNN2j7; Tue, 17 Mar 2009 11:57:44 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 42C352BAC42; Tue, 17 Mar 2009 11:57:44 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id CB935F5CFA; Tue, 17 Mar 2009 08:57:43 -0700 (PDT) Date: Tue, 17 Mar 2009 16:12:00 -0000 From: Joel Brobecker To: don.lee@sunplusct.com Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Support new target S+core 3, a variant of S+core Message-ID: <20090317155743.GA31979@adacore.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.18 (2008-05-17) Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-03/txt/msg00311.txt.bz2 > The S+core team in our company have ported GDB to S+core 3, a variant > of S+core, now we want to contribute some patch. So here it is (see > attachment). Thanks, Don. > Please review it, and any suggestions are most welcome. Note that Qinwei is also capable of reviewing your code. I can take a look, but it's better if Qinwei does it, because he has actual knowledge of the target. > At now, I'm quite new to GDB, so if you fond something I did wrong, > please point it out. If you haven't done so, the gdb/CONTRIBUTE file contains valuable information. You may also want to give gdb/MAINTAINERS a quick read. The first thing that struck me was the fact that the patch was produced in the wrong format, so "context" is missing. Could you resubmit your patch using the "-u" option? For instance "cvs diff -u ...". Also, it looks like you are submitting several changes in one patch, which we prefer not to. For instance, your change to gdb/configure.tgt can be submitted as is, as it is independent of the target; it just tells gdb about the the simulator. > --- src/gdb/MAINTAINERS 2009-03-04 10:26:35.000000000 +0800 > +++ src.mod/gdb/MAINTAINERS 2009-03-04 10:29:15.000000000 +0800 > @@ -307,6 +307,7 @@ > > score --target=score-elf > Qinwei qinwei@sunnorth.com.cn > + Don Lee don.lee@sunplusct.com > > sh --target=sh-elf ,-Werror > --target=sh64-elf ,-Werror As I said on the gdb@ mailing-list, I would prefer to hold off on that change, but others might have a different opinion. In any case, this is also an independent change, so this can be submitted separately from the rest. > Index: gdb/configure.tgt > =================================================================== > RCS file: /cvs/src/src/gdb/configure.tgt,v > retrieving revision 1.213 > diff -r1.213 configure.tgt > 378c378,382 > < gdb_target_obs="score-tdep.o" > --- > > gdb_target_obs="score-tdep.o corelow.o" > > if test -f ../sim/score/Makefile; then > > gdb_sim=../sim/score/libsim.a > > fi > > build_gdbserver=yes There is no score subdirectory in sim/. Are you going to submit a simulator for that target? The rest of the patch is very hard to read! Could you explain a little bit the actual changes you made? At first sight, what the patch says is that you replaced pretty much everything, which I doubt is the case. -- Joel