Re: [patch 1/9]#2 Rename `enum target_signal' to target_signal_t

Pedro Alves
2010-09-01T20:30:14Z

On Wednesday 01 September 2010 21:10:25, Jan Kratochvil wrote:
> On Wed, 01 Sep 2010 21:59:36 +0200, Pedro Alves wrote:
> > Anyway, personally, I'd just do a enum target_signal/enum gdb_signal
> > rename, and stay with that.
>
> I do not mind about this point, someone can post it.

I thought you were already going to have to rename target_signal_t
anyway for your patch to be acceptible, as it has been raised that
target_signal_t was not a good name, given that it ends in _t.
Sorry if I misunderstood that.

> Now I thought it is reusable at least as a code cleanup when it is already in
> IMO an acceptable state.

I personally didn't think things looked that cleaner afterwards, but I
do recognize the value in catching bugs at compile time, hence I'll
agree that that justifies code not looking as neat in some cases. I
was just giving out a suggestion that I thought might be interesting. I
am really sorry that you go through all this trouble of writing large
mechanical patches that may sometimes be dropped. But I do believe that you
can save yourself effort sometimes by discussing these changes upfront,
or just post an experimental patch that doesn't fix everything all
uses at once. That said, once again, I have not objected to your patch.

> I also found as an advantage target_signal_t as a struct could get extended by
> additional information later, as has been done now (although unnecessarily
> - not completely wrongly) for siginfo, addressing the "gdb/signals.h" part:
>
> However, it is
> recognized that this set of signals has limitations (such as not
> distinguishing between various kinds of SIGSEGV, or not
> distinguishing hitting a breakpoint from finishing a single step).
> So in the future we may get around this either by adding additional
> signals for breakpoint, single-step, etc., or by adding signal
> codes; the latter seems more in the spirit of what BSD, System V,
> etc. are doing to address these issues. */

I see no reason that we can't extend target_waitkind/target_waitstatus
instead if we need to. There's no implying from the above that the
"signal codes" need to be the target_signal itself. I think that may
be talking about siginfo->si_code even.

--
Pedro Alves