Re: [PATCH] gdb: make "stop-on-solib-events" a boolean setting

[Andrew Burgess <aburgess@redhat.com>]

Simon Marchi <simon.marchi@efficios.com> writes:

> While reviewing a patch, I noticed that the "stop-on-solib-events"
> setting was an integer, even though it is spiritually a boolean.  It has
> been an integer since it was introduced in commit 87273c713f0e
> (1996-01-23).  Make it a boolean setting.
>
> User-visible improvements:
>
>  - The setting accepts the usual boolean values: on/off, yes/no,
>    enable/disable, as well as 1/0.
>
>  - "set stop-on-solib-events" with no argument now turns the setting
>    on, like other boolean settings, instead of erroring out with
>    "Argument required (integer to set it to)".
>
>  - Tab completion offers "on" and "off".
>
>  - "show stop-on-solib-events" prints "on" or "off" instead of a
>    number.
>
> Possible breaking changes:
>
>  - Integer values other than 0 and 1, as well as expressions (for
>    instance "set stop-on-solib-events $var"), are now rejected with an
>    error.
>
>  - The CLI and MI output changes from:
>
>      (gdb) show stop-on-solib-events
>      Stopping for shared library events is 0.
>      (gdb) interpreter-exec mi "-gdb-show stop-on-solib-events"
>      ^done,value="0"
>      (gdb) print $_gdb_setting_str("stop-on-solib-events")
>      $1 = "0"
>
>    to:
>
>      (gdb) show stop-on-solib-events
>      Stopping for shared library events is off.
>      (gdb) interpreter-exec mi "-gdb-show stop-on-solib-events"
>      ^done,value="off"
>      (gdb) print $_gdb_setting_str("stop-on-solib-events")
>      $1 = "off"
>
>  - The Python gdb.parameter("stop-on-solib-events") function now returns
>    True or False instead of an integer.  The Guile equivalent now
>    returns #t or #f.
>
> I think that all these observable changes are relatively easy to work
> around.
>
> Add a NEWS entry for the change and update the documentation.

I agree that this is a good improvement to make.  I have two minor
nits, see below, but otherwise:

Approved-By: Andrew Burgess <aburgess@redhat.com>

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index a24f67cb8de1..5cfbad231402 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -22510,7 +22510,7 @@ The surrounding square brackets are optional.
>  @end table
>  
>  @table @code
> -@item set stop-on-solib-events
> +@item set stop-on-solib-events [on|off]

This should use "@r{[}on@r{|}off@r{]}" which formats the [|] in normal
non-code style text to make it distinct from the on and off, which will
use @code style.

Our docs are super inconsistent and I have a patch to try and clean this
up but I was waiting for the release to be out of the way before posting
it.

>  @kindex set stop-on-solib-events
>  This command controls whether @value{GDBN} should give you control
>  when the dynamic linker notifies it about some shared library event.
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 3fbfeff9269d..59c25259fd52 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -413,9 +413,9 @@ get_detach_signal (process_stratum_target *proc_target, ptid_t ptid)
>  
>  static struct cmd_list_element *stop_command;
>  
> -/* Nonzero if we want to give control to the user when we're notified
> +/* True if we want to give control to the user when we're notified
>     of shared library events by the dynamic linker.  */
> -int stop_on_solib_events;
> +bool stop_on_solib_events;

The comment on this should really be: /* See infrun.h.  */

Thanks,
Andrew