From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32440 invoked by alias); 8 Dec 2004 22:20:06 -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 32094 invoked from network); 8 Dec 2004 22:19:59 -0000 Received: from unknown (HELO sibelius.xs4all.nl) (82.92.89.47) by sourceware.org with SMTP; 8 Dec 2004 22:19:59 -0000 Received: from elgar.sibelius.xs4all.nl (elgar.sibelius.xs4all.nl [192.168.0.2]) by sibelius.xs4all.nl (8.13.0/8.13.0) with ESMTP id iB8MJrhE025336; Wed, 8 Dec 2004 23:19:53 +0100 (CET) Received: from elgar.sibelius.xs4all.nl (localhost [127.0.0.1]) by elgar.sibelius.xs4all.nl (8.12.6p3/8.12.6) with ESMTP id iB8MJrmR008809; Wed, 8 Dec 2004 23:19:53 +0100 (CET) (envelope-from kettenis@elgar.sibelius.xs4all.nl) Received: (from kettenis@localhost) by elgar.sibelius.xs4all.nl (8.12.6p3/8.12.6/Submit) id iB8MJi18008804; Wed, 8 Dec 2004 23:19:44 +0100 (CET) Date: Thu, 09 Dec 2004 00:52:00 -0000 Message-Id: <200412082219.iB8MJi18008804@elgar.sibelius.xs4all.nl> From: Mark Kettenis To: kewarken@qnx.com CC: gdb-patches@sources.redhat.com In-reply-to: <41B47B26.2060905@qnx.com> (message from Kris Warkentin on Mon, 06 Dec 2004 10:30:46 -0500) Subject: Re: Revised: [patch] general updates and improvements to QNX NTO support References: <419E5B3C.3020209@qnx.com> <41A794A8.9090008@qnx.com> <41B0905D.7070408@qnx.com> <200412031617.iB3GHFN3020509@elgar.sibelius.xs4all.nl> <41B0988D.3030005@qnx.com> <200412031743.iB3Hhr2Y020661@elgar.sibelius.xs4all.nl> <41B47B26.2060905@qnx.com> X-SW-Source: 2004-12/txt/msg00244.txt.bz2 Date: Mon, 06 Dec 2004 10:30:46 -0500 From: Kris Warkentin Based on Mark's comments, I've refactored this patch to provide a cleaner interface to nto_tdep.c. I created an nto_set_target function so that the targets can initialize their own nto_target_ops and set the target properly rather than initializing current_nto_target. Thanks. That looks good! I still think you should avoid the #define nto_xxx_yyy (current_nto_target.xxx_yyy) defines. They're hiding the fact that you're using a global variable. Consider a patch that uses current_nto_target.xxx_yyy directly pre-approved. I've got one additional nit, please see below. I thought about completely hiding the implementation by turning all the macros into functions which just call the current target functions but I'm not sure if it's worth it. Specifically I'm thinking about cases where other modules need access to members of current_nto_target. For instance, the remote and procfs modules need to set various flags and hooks so if I were to hide current_nto_target, I'd need setter/getter functions. Certainly that is the cleaner 'OO' approach. If it needs changing later, I might also store a list of targets in nto-tdep.c. It doesn't seem like a high priority right now though since we probably won't be multi-arching our gdb any time soon. This can be done when the need arises. +enum gdb_osabi +nto_elf_osabi_sniffer (bfd *abfd) +{ + if (nto_is_nto_target) + { + return nto_is_nto_target (abfd); + } + return GDB_OSABI_UNKNOWN; +} + Could you remove the redundant braces here? With that change, this is OK. Thanks, Mark