From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17742 invoked by alias); 9 Mar 2003 04:34:09 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 17735 invoked from network); 9 Mar 2003 04:34:09 -0000 Received: from unknown (HELO hub.ott.qnx.com) (209.226.137.76) by 172.16.49.205 with SMTP; 9 Mar 2003 04:34:09 -0000 Received: from smtp.ott.qnx.com (smtp.ott.qnx.com [10.0.2.158]) by hub.ott.qnx.com (8.9.3/8.9.3) with ESMTP id XAA11755; Sat, 8 Mar 2003 23:20:57 -0500 Received: from dash ([192.168.20.27]) by smtp.ott.qnx.com (8.8.8/8.6.12) with SMTP id XAA08992; Sat, 8 Mar 2003 23:34:07 -0500 Message-ID: <001c01c2e5f5$c591a540$2a00a8c0@dash> From: "Kris Warkentin" To: "Mark Kettenis" Cc: References: <200303081344.h28DiNvi040785@elgar.kettenis.dyndns.org> <004001c2e584$65814690$2a00a8c0@dash> <200303090013.h290DnBU000820@elgar.kettenis.dyndns.org> Subject: Re: [RFC] QNX Neutrino/i386 support Date: Sun, 09 Mar 2003 04:34:00 -0000 MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit X-Priority: 3 X-MSMail-Priority: Normal X-MIMEOLE: Produced By Microsoft MimeOLE V6.00.2800.1106 X-SW-Source: 2003-03/txt/msg00201.txt.bz2 > * Your files should include the blurb about the GPL. Look at the > files that are already part of GDB. I had just looked at the vxworks files. I should have looked at a few others. > #ifdef USG > #include > #endif > > >>> We shouldn't introduce any new uses of the USG define. What should I check for? Is there a HAVE_SYS_TYPES define or some such? > extern unsigned nto_cpuinfo_flags; > extern int nto_cpuinfo_valid; > extern int nto_ostype, nto_cputype; > > >>> Are these global variables really needed? If they are, they > >>> probably belong in a header file of some sort, and they would > >>> deserve a comment. Actually, they're in nto-tdep.h. I just missed removing the externs. As far as whether they're needed, they just help us work around a few ugly issues like whether the remote supports fxsave or not. Also used to figure out remote targets when using qnet, our "transparent" networking protocol where you could do something like 'target procfs /net/some_machine'. There are probably better ways of doing these things. That's where you guys come in - to help me remove hacks. ;-) I'll definitely make some comments. > else > { > regset = -1; /* error */ > } > return regset; > } > > >>> Personally, I would leave out the braces here. Eew. Good thing I didn't write that function. ;-) There have been many fingers in these pies and clean up is not fun. You should see our mips and sh support.... > supply_register (first_regno, (char *) &minusone); > > >>> GDB's convention is to fill registers with zero instead of -1. At > >>> least that's what all the other i386 targets do. I think that the -1 might have been from a long long time ago. These files were originally from gdb 4.17 or earlier. > /* If remote-nto-i386.c calls nto_supply_register in remote-nto.c, > * then remote-nto.c calls nto_cpu_register_fetch. Since this never > * happens, this function is left unimplemented. */ > >>> This comment references remote-i386-nto.c. There is no such file > >>> in your patch. My register refactoring eliminated this function last week. The register refactoring is on our head branch which I need to port to my FSF submission branch....some of it also needs to be back ported to our previous release branch.... Arghh....too many trees make Kris go crazy.... > /* setjmp()'s return PC saved in EDX (5) */ > tdep->jb_pc_offset = 5 * sizeof (int); > > >>> You cannot use sizeof(int) here, since that might very well differ > >>> from 4 on other systems. Ooh...good catch. That might spank me on sparc64. > /* After a watchpoint trap, the PC points to the instruction after > the one that caused the trap. Therefore we don't need to step over it. > But we do need to reset the status register to avoid another trap. */ > HAVE_CONTINUABLE_WATCHPOINT = 1; > > >>> This HAVE_CONTINUABLE_WATCHPOINT stuff has been moved to the > >>> target vector, so that's where you should provide the appropriate > >>> definition. I did the target vector patch but I guess I don't know where this should have gone...Help? > /* read our extra gdbinit file */ > nto_source_extra_gdbinit (".ntox86-gdbinit"); > > >>> This extra-gdbinit-stuff doesn't belong here. We have had a > >>> discussion about these target-dependent init files recently, and I > >>> believe the outcome was that if they were wanted, some general > >>> mechanism should be added rather than these kind of ad-hoc hacks. This is not a critical feature and can be removed for an FSF submission pending a better way of doing it. I could volunteer to try and find one. Thanks for the commentary Mark. I'm on semi-vacation next week (AFK Monday, Wednesday, Friday) but I'll try to keep this flowing. cheers, Kris