[0/4] Unify string-reading APIs

Message ID 20200612215356.22145-1-tromey@adacore.com
Headers show
Series
  • Unify string-reading APIs
Related show

Message

Tom Tromey June 12, 2020, 9:53 p.m.
An old comment notes that gdb has several different ways to read a
string from the target.  This series tries to simplify this situation.

read_memory_string is removed entirely.  Perhaps this is the better
name, in the end; but I didn't want to move read_string into
corelow.c.  Maybe we need a new file for the memory-reading functions
that are there?  (Also I wonder if any of the remaining ones are
redundant.)

target_read_string is first rewritten in terms of read_string.  A
subsequent patch changes its API to be simpler.

Perhaps further simplification could be done as well.  I'm open to
suggestions.

Regression tested on x86-64 Fedora 30.  I also built it using a mingw
cross, to make sure windows-nat.c still builds.

Tom

Comments

Simon Marchi June 13, 2020, 2:40 p.m. | #1
On 2020-06-12 5:53 p.m., Tom Tromey wrote:
> An old comment notes that gdb has several different ways to read a

> string from the target.  This series tries to simplify this situation.

> 

> read_memory_string is removed entirely.  Perhaps this is the better

> name, in the end; but I didn't want to move read_string into

> corelow.c.  Maybe we need a new file for the memory-reading functions

> that are there?  (Also I wonder if any of the remaining ones are

> redundant.)

> 

> target_read_string is first rewritten in terms of read_string.  A

> subsequent patch changes its API to be simpler.

> 

> Perhaps further simplification could be done as well.  I'm open to

> suggestions.

> 

> Regression tested on x86-64 Fedora 30.  I also built it using a mingw

> cross, to make sure windows-nat.c still builds.

> 

> Tom


Apart from the few stylistic comments I've made, it LGTM.

Thanks!

Simon
Tom Tromey June 15, 2020, 12:27 p.m. | #2
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:


Simon> Apart from the few stylistic comments I've made, it LGTM.

I fixed these up, and I'm checking this in now.

Tom