Re: symbolic debug of loadable modules with kgdb light

[Eli Zaretskii <eliz@gnu.org>]

> From: Caz Yokoyama <cazyokoyama@gmail.com>
> Cc: "'Pedro Alves'" <pedro@codesourcery.com>, 	<gdb-patches@sourceware.org>
> Date: Tue, 29 Sep 2009 21:45:46 -0700
> 
> Here is the patch for 1. Let me know if I forget something. Also any
> comments are welcome. Thank you.

Thanks.

> --- gdb/NEWS	15 Sep 2009 03:30:04 -0000	1.331
> +++ gdb/NEWS	30 Sep 2009 04:41:52 -0000
> @@ -3,6 +3,15 @@
>  
>  *** Changes since GDB 6.8
>  
> +* "set/show remotebreak" command is deprecated. "set/show remote interrupt-sequence"
> +is added. They add break signal followed by a character 'g' in addition to control-c
> +and a break signal. break signal and g is also known as Magic SysRq and it interrupts
> +Linux kernel.
> +
> +* "set/show remote interrupt-on-start" command is added. When this is ON,
> +gdb sends interrupt-sequence to the remote target when gdb starts. This is needed when
> +you debug Linux kernel.

Several comments about the changes in NEWS:

  . Please put this in the "New commands" section.

  . Please format it like the other entries about new commands.

  . Please use shorter lines, like no more than 70 characters.

  . Please leave two spaces between sentences.

> +  add_setshow_enum_cmd ("interrupt-sequence", class_support,
> +			interrupt_sequence_modes, &interrupt_sequence_mode, _("\
> +Set interrupt sequence to remote target, control-c/break/sysrq-g."), _("\

You cannot have commas on the first line of the doc string, because
commands that show only the first line will stop at the first comma.

> --- gdb/doc/ChangeLog	26 Sep 2009 16:47:13 -0000	1.958
> +++ gdb/doc/ChangeLog	30 Sep 2009 04:41:58 -0000

Several comments about the patch to the manual:

+2009-09-29  Kazuyoshi Caz Yokoyama  <caz@caztech.com>
+
+	* gdb.texinfo: remove "set/show remotebreak" command.
+	Add "set/show remote interrupt-sequence" and
+	"set/show remote interrupt-on-start" command.
+

ChangeLog entries should begin with a capital letter, and should
mention the name of the node where you make changes (as if the node
were a function in some programming language).

> -@item set remotebreak
> -@cindex interrupt remote programs
> -@cindex BREAK signal instead of Ctrl-C
> -@anchor{set remotebreak}
> -If set to on, @value{GDBN} sends a @code{BREAK} signal to the remote
> -when you type @kbd{Ctrl-c} to interrupt the program running
> -on the remote.  If set to off, @value{GDBN} sends the @samp{Ctrl-C}
> -character instead.  The default is off, since most remote systems
> -expect to see @samp{Ctrl-C} as the interrupt signal.
> -
> -@item show remotebreak
> -Show whether @value{GDBN} sends @code{BREAK} or @samp{Ctrl-C} to
> -interrupt the remote program.
> -
>  @item set remoteflow on
>  @itemx set remoteflow off
>  @kindex set remoteflow
> @@ -15011,6 +14997,34 @@
>  target system.  If it is not set, the target will use a default
>  filename (e.g.@: the last program run).
>  
> +@item set interrupt-sequence
> +@cindex interrupt remote programs
> +@cindex select control-c, break or sysrq-g

Please use the same conventions for these keys as in the portion you
are replacing.  For example, BREAK should be in caps and control-c
should be spelled Ctrl-C.

> +@anchor{set interrupt-sequence}

Why are you setting up an @anchor?  I don't see that you use it
anywhere in the patch.  An @anchor is for use in a cross-reference,
like @xref or @pxref.

> +Allow the user to specify what sequence to send to the remote target
> +when he requests a program interruption: Although @kbd{control-c} is usually
> +what remote systems expect (this is the default, here), it is
> +sometimes preferable to send a @code{break}.

This description leaves the reader in the dark what she should or
could type to set the interrupt key sequence.  Will a literal
"control-c" be acceptable? what about "Ctrl-C"? is the string
case-sensitive or not? what sequences are recognized and supported?
Or do I just press the key sequence itself, i.e. press and hold Ctrl
and hit C, or hit BREAK then g?

>  On other systems such as the Linux kernel

Linux kernel is not a system.  Please use "On other systems such as
GNU/Linux" or "when debugging the Linux kernel".

> +@anchor{set interrupt-on-start}

This anchor is also unused.

> +Specify whether interrupt_sequence is sent
                   ^^^^^^^^^^^^^^^^^^
Why with an underscore?

> +to remote target when gdb starts. This is mostly needed when you debug

Please use @value{GDBN}" instead of a literal "gdb".

> +@code{Linux kernel}. Linux kernel expects @code{BREAK g} which is @code{Magic SysRq}
                      ^^
Two spaces between sentences, please.

Also, keyboard input should have the @kbd markup: @kbd{@key{BREAK} g}.
Note that I used @key{BREAK} to indicate that this is a single key,
not the sequence of characters B R E A K.

> +setting (@pxref{set remote interrupt-sequence}).

I don't think this @pxref will work.  Did you try to say "make info"
after changing gdb.texinfo, and if so, did it complain?