[0/5] Fixes for per-inferior settings and $_gdb_setting_str()

Message ID cover.1680608960.git.aburgess@redhat.com
Headers
Series Fixes for per-inferior settings and $_gdb_setting_str() |

Message

Andrew Burgess April 4, 2023, 12:45 p.m. UTC
  While working on another patch I spotted that
$_gdb_setting_str("args") wasn't working correctly.  Turns out args,
cwd, and inferior-tty all have the same problems.  This series
resolves these issues.

Thanks,
Andrew

---

Andrew Burgess (5):
  gdb: cleanup command creation in infcmd.c
  gdb: make set/show args work with $_gdb_setting_str
  gdb: make set/show cwd work with $_gdb_setting_str
  gdb: make set/show inferior-tty work with $_gdb_setting_str
  gdb: make deprecated_show_value_hack static

 gdb/cli/cli-setshow.c                     |   2 +-
 gdb/command.h                             |   7 +-
 gdb/infcmd.c                              | 127 ++++++++++------------
 gdb/testsuite/gdb.base/inferior-clone.exp |   9 ++
 gdb/testsuite/gdb.multi/gdb-settings.c    |  22 ++++
 gdb/testsuite/gdb.multi/gdb-settings.exp  | 121 +++++++++++++++++++++
 6 files changed, 213 insertions(+), 75 deletions(-)
 create mode 100644 gdb/testsuite/gdb.multi/gdb-settings.c
 create mode 100644 gdb/testsuite/gdb.multi/gdb-settings.exp


base-commit: 60a13bbcdfb0ce008a77563cea0c34c830d7b170
  

Comments

Tom Tromey April 17, 2023, 4:42 p.m. UTC | #1
>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> While working on another patch I spotted that
Andrew> $_gdb_setting_str("args") wasn't working correctly.  Turns out args,
Andrew> cwd, and inferior-tty all have the same problems.  This series
Andrew> resolves these issues.

I sent a minor comment but nothing really worth a repost.

Reviewed-By: Tom Tromey <tom@tromey.com>

thank you,
Tom
  
Simon Marchi April 17, 2023, 6:09 p.m. UTC | #2
On 4/4/23 08:45, Andrew Burgess via Gdb-patches wrote:
> While working on another patch I spotted that
> $_gdb_setting_str("args") wasn't working correctly.  Turns out args,
> cwd, and inferior-tty all have the same problems.  This series
> resolves these issues.

That rang a bell, it's because I have fixed some of these downstream in
ROCgdb, and haven't sent it upstream yet.  The downstream commit is
here:

https://github.com/ROCm-Developer-Tools/ROCgdb/commit/54d0a79614071fd62df5e2dfe793ff26cc7e31e6

I'll take a look at your patches, I presume that they will do mostly the
same thing.  And if my patch does more (according to the commit message,
it fixes "remote exec-file" and "tdesc filename"), I can send what
remains after.

Simon
  
Simon Marchi April 17, 2023, 6:21 p.m. UTC | #3
On 4/17/23 14:09, Simon Marchi via Gdb-patches wrote:
> On 4/4/23 08:45, Andrew Burgess via Gdb-patches wrote:
>> While working on another patch I spotted that
>> $_gdb_setting_str("args") wasn't working correctly.  Turns out args,
>> cwd, and inferior-tty all have the same problems.  This series
>> resolves these issues.
> 
> That rang a bell, it's because I have fixed some of these downstream in
> ROCgdb, and haven't sent it upstream yet.  The downstream commit is
> here:
> 
> https://github.com/ROCm-Developer-Tools/ROCgdb/commit/54d0a79614071fd62df5e2dfe793ff26cc7e31e6
> 
> I'll take a look at your patches, I presume that they will do mostly the
> same thing.  And if my patch does more (according to the commit message,
> it fixes "remote exec-file" and "tdesc filename"), I can send what
> remains after.

So, my patch adds some tests for gdb.parameter (Python), MI and the with
command.  However, they are sprinkled in different files.  I like that
you have a test file specifically to exercise the settings that are
per-inferior.  Your test could probably be augmented to exercise all
these ways to get a setting.

You series looks good to me, up to you if you want to merge it as-is or
improve it based on what I had.

Simon
  
Andrew Burgess April 28, 2023, 4:43 p.m. UTC | #4
Simon Marchi <simark@simark.ca> writes:

> On 4/4/23 08:45, Andrew Burgess via Gdb-patches wrote:
>> While working on another patch I spotted that
>> $_gdb_setting_str("args") wasn't working correctly.  Turns out args,
>> cwd, and inferior-tty all have the same problems.  This series
>> resolves these issues.
>
> That rang a bell, it's because I have fixed some of these downstream in
> ROCgdb, and haven't sent it upstream yet.  The downstream commit is
> here:
>
> https://github.com/ROCm-Developer-Tools/ROCgdb/commit/54d0a79614071fd62df5e2dfe793ff26cc7e31e6

In case I miss this patch when you upstream it, the removal of
show_remote_exec_file should be dropped.  The generic code doesn't
handle internationalisation, and though show_remote_exec_file itself
also doesn't do internationalisation, the correct (IMHO) thing would be
to fix show_remote_exec_file, not remove it.

>
> I'll take a look at your patches, I presume that they will do mostly the
> same thing.  And if my patch does more (according to the commit message,
> it fixes "remote exec-file" and "tdesc filename"), I can send what
> remains after.

I'll take a look at the testing in your patch and see if I can extend
the tests in my patch a little.

Thanks,
Andrew
  
Andrew Burgess April 28, 2023, 9:53 p.m. UTC | #5
Simon Marchi <simark@simark.ca> writes:

> On 4/17/23 14:09, Simon Marchi via Gdb-patches wrote:
>> On 4/4/23 08:45, Andrew Burgess via Gdb-patches wrote:
>>> While working on another patch I spotted that
>>> $_gdb_setting_str("args") wasn't working correctly.  Turns out args,
>>> cwd, and inferior-tty all have the same problems.  This series
>>> resolves these issues.
>> 
>> That rang a bell, it's because I have fixed some of these downstream in
>> ROCgdb, and haven't sent it upstream yet.  The downstream commit is
>> here:
>> 
>> https://github.com/ROCm-Developer-Tools/ROCgdb/commit/54d0a79614071fd62df5e2dfe793ff26cc7e31e6
>> 
>> I'll take a look at your patches, I presume that they will do mostly the
>> same thing.  And if my patch does more (according to the commit message,
>> it fixes "remote exec-file" and "tdesc filename"), I can send what
>> remains after.
>
> So, my patch adds some tests for gdb.parameter (Python), MI and the with
> command.  However, they are sprinkled in different files.  I like that
> you have a test file specifically to exercise the settings that are
> per-inferior.  Your test could probably be augmented to exercise all
> these ways to get a setting.
>
> You series looks good to me, up to you if you want to merge it as-is or
> improve it based on what I had.

I reworked the tests taking onboard some of the ideas from your tests,
and pushed this series.

Do you plan to upstream your patch anytime soon?  I had been planning to
look at other per-inferior settings once this series was merged, but I
don't want to tread on your toes any more than I already have...

Thanks,
Andrew