Re: symbolic debug of loadable modules with kgdb light

Eli Zaretskii
2009-09-30T19:16:37Z

> 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?