Re: [RFC] "single step" atomic instruction sequences as a whole on PPC

["Ulrich Weigand" <uweigand@de.ibm.com>]

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