From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16051 invoked by alias); 23 Jun 2007 08:05:07 -0000 Received: (qmail 16043 invoked by uid 22791); 23 Jun 2007 08:05:06 -0000 X-Spam-Check-By: sourceware.org Received: from romy.inter.net.il (HELO romy.inter.net.il) (213.8.233.24) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 23 Jun 2007 08:05:03 +0000 Received: from HOME-C4E4A596F7 (IGLD-84-229-247-82.inter.net.il [84.229.247.82]) by romy.inter.net.il (MOS 3.7.3-GA) with ESMTP id IDP48343 (AUTH halo1); Sat, 23 Jun 2007 11:04:32 +0300 (IDT) Date: Sat, 23 Jun 2007 08:05:00 -0000 Message-Id: From: Eli Zaretskii To: Markus Deuling CC: gdb-patches@sourceware.org, uweigand@de.ibm.com In-reply-to: <467B7557.9000708@de.ibm.com> (message from Markus Deuling on Fri, 22 Jun 2007 09:08:07 +0200) Subject: Re: [rfc] Replace macros by gdbarch functions in gdbint manual Reply-to: Eli Zaretskii References: <4678FEBE.7040209@de.ibm.com> <467B7557.9000708@de.ibm.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: 2007-06/txt/msg00432.txt.bz2 > Date: Fri, 22 Jun 2007 09:08:07 +0200 > From: Markus Deuling > CC: gdb-patches@sourceware.org, uweigand@de.ibm.com > > thank you for your review. I reworked the patch and think I addressed all of your points. Thanks. Unfortunately, this is not yet done, see below. I apologize if I failed to make some of these comments the first time you posted your original patch: it's not easy to see minor problems in such a large patch. > The section "Converting an existing Target Architecture to Multi-arch" seems to be obsolete? I don't know enough about this. Daniel, could you comment, please? > -address of the instruction. ADDR_BITS_REMOVE should filter out these > +address of the instruction. gdbarch_addr_bits_remove should filter out these > bits with an expression such as @code{((addr) & ~3)}. gdbarch_addr_bits_remove should be in @code, as it is a C symbol. (Yes, I know: the original text had that mistake as well.) > -@item CANNOT_FETCH_REGISTER (@var{regno}) > -@findex CANNOT_FETCH_REGISTER > +@item int gdbarch_cannot_fetch_register (@var{gdbarch}, @var{regum}) > +@findex gdbarch_cannot_fetch_register > A C expression that should be nonzero if @var{regno} cannot be fetched > from an inferior process. This is only relevant if > @code{FETCH_INFERIOR_REGISTERS} is not defined. gdbarch_cannot_fetch_register is a function now, so it sounds like saying it's ``a C expression'' would be a mistake. A macro can be an expression, but a function cannot. There are other similar wording problems; search for "C expression". > +stack top) stack address @var{rhs}. Let the function return @code{lhs < rhs} ^^^ Two spaces after a period that ends a sentence, please (here and elsewhere). > +@item int gdbarch_in_solib_return_trampoline (@var{gdbarch}, @var{pc}, @var{name}) > +@findex gdbarch_in_solib_return_trampoline > +Declare this function to evaluate to nonzero if the program is stopped in the > trampoline that returns from a shared library. You repeatedly use "declare a function" where the previous text said "define a macro". I don't think that the use of ``declare'' here is correct: a declaration of a function is its prototype; you really want to say "define a function", which in C parlance means write the function's implementation. > +Return the name of register @var{regnr} as a string. May return @code{NULL} > +to indicate that register @var{regnr} is not valid. A minor stylistic point: "indicate that @var{regnr} is not a valid register" sounds better. > -@item SKIP_PERMANENT_BREAKPOINT > -@findex SKIP_PERMANENT_BREAKPOINT > +@item void gdbarch_skip_permanent_breakpoint (@var{gdbarch}, @var{regcache}) > +@findex gdbarch_skip_permanent_breakpoint > Advance the inferior's PC past a permanent breakpoint. @value{GDBN} normally > steps over a breakpoint by removing it, stepping one instruction, and > re-inserting the breakpoint. However, permanent breakpoints are > hardwired into the inferior, and can't be removed, so this strategy > -doesn't work. Calling @code{SKIP_PERMANENT_BREAKPOINT} adjusts the processor's > -state so that execution will resume just after the breakpoint. This > -macro does the right thing even when the breakpoint is in the delay slot > +doesn't work. Calling @code{gdbarch_skip_permanent_breakpoint} adjusts the > +processor's state so that execution will resume just after the breakpoint. > +This macro does the right thing even when the breakpoint is in the delay slot ^^^^^^^^^^ But this isn't a macro anymore, is it? > +@findex gdbarch_skip_trampoline_code > If the target machine has trampoline code that sits between callers and > -the functions being called, then define this macro to return a new PC > +the functions being called, then set this function to return a new PC > that is at the start of the real function. "set this function" again.