[PATCH] Notify observers that directories have changed when using "directory" CLI command
Andrew Burgess
andrew.burgess@embecosm.com
Fri Oct 2 10:03:58 GMT 2020
Hi,
Thanks for working on this. This does look like a good change,
however, I have a couple of questions...
* Jan Vrany via Gdb-patches <gdb-patches@sourceware.org> [2020-10-01 13:52:47 +0100]:
> gdb/ChangeLog
>
> * source.c (directory_command): Notify observers that "directories"
> parameter has changed.
>
> gdb/testsuite/ChangeLog
>
> * gdb.mi/mi-cmd-param-changed.exp: Check that notification is
> is emmited for both 'set directories' and 'directory' commands.
> ---
> gdb/ChangeLog | 5 +++++
> gdb/source.c | 1 +
> gdb/testsuite/ChangeLog | 5 +++++
> gdb/testsuite/gdb.mi/mi-cmd-param-changed.exp | 8 ++++++++
> 4 files changed, 19 insertions(+)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 9c802c57ef..259cdd1981 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2020-10-01 Jan Vrany <jan.vrany@labware.com>
> +
> + * source.c (directory_command): Notify observers that "directories"
> + parameter has changed.
> +
> 2020-09-29 Tom Tromey <tom@tromey.com>
>
> * dwarf2/read.c (lookup_dwo_id, get_type_unit_group)
> diff --git a/gdb/source.c b/gdb/source.c
> index 0c2b5a4f83..131aebb09b 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -460,6 +460,7 @@ directory_command (const char *dirname, int from_tty)
> {
> mod_path (dirname, &source_path);
> forget_cached_source_info ();
> + gdb::observers::command_param_changed.notify ("directories", source_path);
I wonder why you only notify in this one case, I've copied the whole
of directory_command below and added an annotation for where I think a
second notification could/should go:
static void
directory_command (const char *dirname, int from_tty)
{
dont_repeat ();
/* FIXME, this goes to "delete dir"... */
if (dirname == 0)
{
if (!from_tty || query (_("Reinitialize source path to empty? ")))
{
xfree (source_path);
init_source_path ();
/* Why not notify here too? */
}
}
else
{
mod_path (dirname, &source_path);
forget_cached_source_info ();
gdb::observers::command_param_changed.notify ("directories", source_path);
}
if (from_tty)
show_directories_1 ((char *) 0, from_tty);
}
I'd be tempted to only have a single call to notify myself, something
like:
static void
directory_command (const char *dirname, int from_tty)
{
bool value_changed = false;
dont_repeat ();
/* FIXME, this goes to "delete dir"... */
if (dirname == 0)
{
if (!from_tty || query (_("Reinitialize source path to empty? ")))
{
xfree (source_path);
init_source_path ();
value_changed = true;
}
}
else
{
mod_path (dirname, &source_path);
forget_cached_source_info ();
value_changed = true;
}
if (value_changed) /* I added this. */
gdb::observers::command_param_changed.notify ("directories", source_path);
if (from_tty && value_changed) /* I changed this too. */
show_directories_1 ((char *) 0, from_tty);
}
> }
> if (from_tty)
> show_directories_1 ((char *) 0, from_tty);
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 2046c9b8e6..f39c8eb7c7 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +2020-10-01 Jan Vrany <jan.vrany@labware.com>
> +
> + * gdb.mi/mi-cmd-param-changed.exp: Check that notification is
> + is emmited for both 'set directories' and 'directory' commands.
> +
> 2020-09-30 Gary Benson <gbenson@redhat.com>
>
> * gdb.dwarf2/dw2-double-set-die-type.S (.Ldie_3e0):
> diff --git a/gdb/testsuite/gdb.mi/mi-cmd-param-changed.exp b/gdb/testsuite/gdb.mi/mi-cmd-param-changed.exp
> index 031d396dba..78795dc293 100644
> --- a/gdb/testsuite/gdb.mi/mi-cmd-param-changed.exp
> +++ b/gdb/testsuite/gdb.mi/mi-cmd-param-changed.exp
> @@ -102,6 +102,14 @@ proc test_command_param_changed { } {
> ".*=cmd-param-changed,param=\"check type\",value=\"on\".*\\^done" \
> "\"set ch type on\""
>
> + # Notification is emitted for both 'set directories' and 'directory'.
> + mi_gdb_test "set directories \$cdir:\$cwd:/tmp" \
> + ".*=cmd-param-changed,param=\"directories\",value=\".*\".*\\^done" \
> + "\"set directories\""
> + mi_gdb_test "directory /usr/src/gdb" \
> + ".*=cmd-param-changed,param=\"directories\",value=\".*\".*\\^done" \
> + "\"directory\""
> +
I'd like to see a test for using `directories` with no arguments too.
Thanks,
Andrew
> mi_gdb_exit
> }
> }
> --
> 2.28.0
>
More information about the Gdb-patches
mailing list