This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [rfc] Replace macros by gdbarch functions in gdbint manual
- From: Eli Zaretskii <eliz at gnu dot org>
- To: Markus Deuling <deuling at de dot ibm dot com>
- Cc: gdb-patches at sourceware dot org, uweigand at de dot ibm dot com
- Date: Sat, 23 Jun 2007 11:04:35 +0300
- Subject: Re: [rfc] Replace macros by gdbarch functions in gdbint manual
- References: <4678FEBE.7040209@de.ibm.com> <u7ipyqxxz.fsf@gnu.org> <467B7557.9000708@de.ibm.com>
- Reply-to: Eli Zaretskii <eliz at gnu dot org>
> Date: Fri, 22 Jun 2007 09:08:07 +0200
> From: Markus Deuling <deuling@de.ibm.com>
> 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.