From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9295 invoked by alias); 12 Apr 2010 16:43:56 -0000 Received: (qmail 9280 invoked by uid 22791); 12 Apr 2010 16:43:54 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=BAYES_00,TW_QS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 12 Apr 2010 16:43:48 +0000 Received: (qmail 1999 invoked from network); 12 Apr 2010 16:43:46 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 12 Apr 2010 16:43:46 -0000 From: Pedro Alves To: "Pierre Muller" Subject: Re: [RFC-v6] Add windows OS Thread Information Block Date: Mon, 12 Apr 2010 16:43:00 -0000 User-Agent: KMail/1.12.2 (Linux/2.6.31-20-generic; KDE/4.3.2; x86_64; ; ) Cc: gdb-patches@sourceware.org References: <000901c9f5ef$4ee06f10$eca14d30$@u-strasbg.fr> <201004111609.54497.pedro@codesourcery.com> <004b01cada47$3f6d6eb0$be484c10$@muller@ics-cnrs.unistra.fr> In-Reply-To: <004b01cada47$3f6d6eb0$be484c10$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201004121743.44687.pedro@codesourcery.com> X-IsSubscribed: yes 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: 2010-04/txt/msg00367.txt.bz2 On Monday 12 April 2010 14:51:27, Pierre Muller wrote: > gdb.texinfo ($__tlb): Document new automatic convinience variable. One _ too much? > Seems good to me, as I anyhow don't think that > Windows OS will ever use '0' as a Thread Information Block address. > Note that this is only a guess, but it might be required > if Windows OS guarantees that the null address is not accessible, > but I don't know if this is indeed written somewhere in the specs. Then don't guess, just remove the check, and we'll add it later if found necessary. Otherwise, it's just bloat and adds to confusion (it makes one stop and wonder when reading that code; I know I have). On Monday 12 April 2010 14:51:27, Pierre Muller wrote: > + return tib_ptr_type; > +} > +/* The $_tlb convenience variable is a bit special. We don't know Missing newline between previous function and following function's comment. Something else I just noticed now: > +/* Display thread information block of a given thread. */ > + > +static int > +display_one_tib (ptid_t ptid) > +{ > +#define PTID_STRING_SIZE 40 > + char annex[PTID_STRING_SIZE]; > + char *annex_end = annex + PTID_STRING_SIZE; > + gdb_byte *tib = NULL; > + gdb_byte *index; > + CORE_ADDR thread_local_base; ... > + if (target_read (¤t_target, TARGET_OBJECT_MEMORY, > + annex, tib, thread_local_base, tib_size) != tib_size) This annex handling also seems something used in a previous patch revision. I think you meant to drop it. Also: > + add_packet_config_cmd (&remote_protocol_packets[PACKET_qGetTIBAddr], > + "qGetTIBAddr", "get-thread-information-block-address", > + 0); this new command should be documented. In addition, all new packets and commands should be mentioned in NEWS. > + if (the_target->get_tib_address != NULL) > + strcat (own_buf, ";qGetTIBAddr+"); You missed addressing a comment I made to this: I can't seem to find where this new qsupported feature was documented in the manual changes? (and is this needed at all? I guess it doesn't hurt.) Also, you have unrelated hunks touching these files: > RCS file: /cvs/src/src/gdb/tui/tui-regs.c,v > RCS file: /cvs/src/src/gdb/tui/tui-stack.c,v please make sure these are not included in the patch. -- Pedro Alves