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

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Thu, 02 Sep 2010 03:54:47 +0200, Pedro Alves wrote:
> A target_signal_o array to hold
> all the possible gdb signals, and a target_signal becomes:
> 
> typedef const struct target_signal_o * target_signal;

This means there is abandoned the possibility to associate dynamic information
with a signal (such as to associate siginfo_t with it).  Target signal (SIG*)
without its siginfo_t is an incomplete information.  Therefore I was
automatically trying to fix this problem before.

Therefore if GDB middle-end (infrun.c) does not need the siginfo_t part ...
does it even need to know the signal number (int host_signal - SIG*) part?
	echo * mi/* tui/* gdbserver/*|tr ' ' '\n'|grep -v ChangeLog|xargs cat|perl -lne 'print $& while /\bTARGET_SIGNAL_\w+/g;'|sort|uniq -c|sort -nr|vi -
says GDB references only 30 TARGET_SIGNAL_* signals (out of the 150 existing
ones) so it does not need to really know the specific signal number.

Those 120 signals could be kept private only to remote.c and gdbserver(s) for
the purpose of compatibility of the gdbserver protocol.
(In fact all TARGET_SIGNAL_* should be kept private to the target as for
example the "Treating signal as SIGTRAP" hack in infrun.c should be target
specific and TARGET_SIGNAL_SIGTRAP should be called
TARGET_SIGNAL_BREAKPOINT_HIT etc.  But a full signals abstraction is a too
expensive/naive/unreal clean-up project for now.)

As a proof Linux `SIGSTKFLT' is now missing in the TARGET_SIGNAL_* list.
But no GDB middle-end code needs to know what is `SIGSTKFLT'.  This suggests
we can reduce the TARGET_SIGNAL_* set only to the 30 signals (for an easy
compatibility with current GDB code) and introduce a new target specific field
(similar to the abandoned siginfo field):

typedef struct
  {
    enum target_signal_number sig;  /* TARGET_SIGNAL_* 30 possibilities */
    int host_signal;  /* untranslated SIG* number for TARGET_SIGNAL_UNKNOWN.  */
  } target_signal;
 - Naming is kept compatible with current GDB naming.  The reality is:
   s/target_signal/gdb_signal/
   s/host_signal/target_signal/

Target could provide a number-name table for the `handle' command covering
just the specific 64 signals available at the Linux target (instead of the 150
ones from the TARGET_SIGNAL_* list).


I don't say I want to do it or that it should be done.  It has just explained
to me why target_signal_t does not need to contain the siginfo_t information.


> Anyway, here it is at least for the archives.

BTW I like this approach (when no dynamic information needs and as explained
above probably never will need to get associated).  I do not like the
`target_signal' name used for a type but that is a nitpick.


<bite>
Is ChangeLog required only for the final check-in?  Or are global maintainers
excluded from the ChangeLog submit requirement?  I have spent a whole day
writing it and the review did not get past the basic idea of my patchset.


Thanks,
Jan