This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] "single step" atomic instruction sequences as a whole on PPC
- From: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- To: luisgpm at linux dot vnet dot ibm dot com
- Cc: gdb-patches at sourceware dot org
- Date: Mon, 7 May 2007 20:11:48 +0200 (CEST)
- Subject: Re: [RFC] "single step" atomic instruction sequences as a whole on PPC
Luis Machado wrote:
> Thanks for the comments. I'm still getting used to the details of the
> coding standard.
>
> The patch has been updated. I hope i've addressed all the coding format
> issues this time.
It's much better, thanks. Unfortunately there's still a number of
issues -- thanks for you patience in dealing with those.
> The handling of multiple conditional branches was modified so that when
> GDB finds such a situation it just silently falls back to the standard
> way of doing single-step. That way we won't have too much text on the
> screen in case this situation happens often.
I guess that's OK with me. I'm wondering why we need the one message
that's still in there then -- I'd say either we warn whenever we find
a sequence we don't understand, or we never warn.
> + /* Assume all atomic sequences start with an lwarx or ldarx instruction. */
> + if ((insn & LWARX_MASK) != LWARX_INSTRUCTION
> + && (insn & LWARX_MASK) != LDARX_INSTRUCTION)
> + return 0;
"return" is now indented two spaces too much.
> + /* Assume that no atomic sequence is longer than "atomic_sequence_length
Missing closing "
> + /* Assume that there is at most one conditional branch in the atomic
> + sequence. If a conditional branch is found, put a breakpoint in
Two spaces after '.'
> + warning (_("Tried to step over an atomic sequence of instructions at %s\n \
> + but could not find the end of the sequence."),
If we keep the warning, we need to remove the extra spaces at the beginning of
the second line. See e.g. _initialize_printcmd for examples of long messages.
> +/* AIX does not support PT_STEP. Simulate it. */
Two spaces not just after the last '.', but after each of them.
> + /* Handles single stepping of atomic sequences */
Comment should end in '.'.
Otherwise, this looks technically OK to me. Once the coding style issues
and the warning question are resolved, I think it can be committed.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com