[v2] Improve load command's help text

Message ID 1483744529-22462-1-git-send-email-lgustavo@codesourcery.com
State New, archived
Headers

Commit Message

Luis Machado Jan. 6, 2017, 11:15 p.m. UTC
  Changes in v2:

- Improved description of how FILE and OFFSET must be passed.
- Complemented description in the GDB manual since it was missing the
  description for OFFSET.

This fairly obvious patch adds usage text to the load command's help text.

Originally it did not have usage and mentioned things like FILE and OFFSET
without explaining how those should be passed in the command.

gdb/ChangeLog:

2017-01-06  Luis Machado  <lgustavo@codesourcery.com>

	* symfile (_initialize_symfile): Add usage text to the load command's
	help text.

gdb/doc/ChangeLog:

2017-01-06  Luis Machado  <lgustavo@codesourcery.com>

	* gdb.texinfo (_initialize_symfile): Document the optional offset
	argument.
---
 gdb/doc/gdb.texinfo | 8 ++++++--
 gdb/symfile.c       | 5 ++++-
 2 files changed, 10 insertions(+), 3 deletions(-)
  

Comments

Luis Machado Jan. 17, 2017, 1:08 p.m. UTC | #1
ping? I suppose this is in good shape now?

On 01/06/2017 05:15 PM, Luis Machado wrote:
> Changes in v2:
>
> - Improved description of how FILE and OFFSET must be passed.
> - Complemented description in the GDB manual since it was missing the
>   description for OFFSET.
>
> This fairly obvious patch adds usage text to the load command's help text.
>
> Originally it did not have usage and mentioned things like FILE and OFFSET
> without explaining how those should be passed in the command.
>
> gdb/ChangeLog:
>
> 2017-01-06  Luis Machado  <lgustavo@codesourcery.com>
>
> 	* symfile (_initialize_symfile): Add usage text to the load command's
> 	help text.
>
> gdb/doc/ChangeLog:
>
> 2017-01-06  Luis Machado  <lgustavo@codesourcery.com>
>
> 	* gdb.texinfo (_initialize_symfile): Document the optional offset
> 	argument.
> ---
>  gdb/doc/gdb.texinfo | 8 ++++++--
>  gdb/symfile.c       | 5 ++++-
>  2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index ccb95ad..d9c77f7 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -19591,8 +19591,8 @@ Show the current status of displaying communications between
>
>  @table @code
>
> -@kindex load @var{filename}
> -@item load @var{filename}
> +@kindex load @var{filename} @var{offset}
> +@item load @var{filename} @var{offset}
>  @anchor{load}
>  Depending on what remote debugging facilities are configured into
>  @value{GDBN}, the @code{load} command may be available.  Where it exists, it
> @@ -19611,6 +19611,10 @@ link the program; for other formats, like a.out, the object file format
>  specifies a fixed address.
>  @c FIXME! This would be a good place for an xref to the GNU linker doc.
>
> +It is also possible to tell @value{GDBN} to load the symbol file at a specific
> +offset described by the optional argument @var{offset}.  When @var{offset} is
> +provided, @var{filename} must also be provided.
> +
>  Depending on the remote side capabilities, @value{GDBN} may be able to
>  load programs into flash memory.
>
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 61b9e5b..6ffc86b 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -3952,7 +3952,10 @@ that lies within the boundaries of this symbol file in memory."),
>    c = add_cmd ("load", class_files, load_command, _("\
>  Dynamically load FILE into the running program, and record its symbols\n\
>  for access from GDB.\n\
> -A load OFFSET may also be given."), &cmdlist);
> +An optional load OFFSET may also be given as a literal address.\n\
> +When OFFSET is provided, FILE must also be provided.  FILE can be provided\n\
> +on its own.\n\
> +Usage: load [FILE] [OFFSET]"), &cmdlist);
>    set_cmd_completer (c, filename_completer);
>
>    add_prefix_cmd ("overlay", class_support, overlay_command,
>
  
Luis Machado Jan. 24, 2017, 4:22 p.m. UTC | #2
ping?

On 01/17/2017 07:08 AM, Luis Machado wrote:
> ping? I suppose this is in good shape now?
>
> On 01/06/2017 05:15 PM, Luis Machado wrote:
>> Changes in v2:
>>
>> - Improved description of how FILE and OFFSET must be passed.
>> - Complemented description in the GDB manual since it was missing the
>>   description for OFFSET.
>>
>> This fairly obvious patch adds usage text to the load command's help
>> text.
>>
>> Originally it did not have usage and mentioned things like FILE and
>> OFFSET
>> without explaining how those should be passed in the command.
>>
>> gdb/ChangeLog:
>>
>> 2017-01-06  Luis Machado  <lgustavo@codesourcery.com>
>>
>>     * symfile (_initialize_symfile): Add usage text to the load command's
>>     help text.
>>
>> gdb/doc/ChangeLog:
>>
>> 2017-01-06  Luis Machado  <lgustavo@codesourcery.com>
>>
>>     * gdb.texinfo (_initialize_symfile): Document the optional offset
>>     argument.
>> ---
>>  gdb/doc/gdb.texinfo | 8 ++++++--
>>  gdb/symfile.c       | 5 ++++-
>>  2 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index ccb95ad..d9c77f7 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -19591,8 +19591,8 @@ Show the current status of displaying
>> communications between
>>
>>  @table @code
>>
>> -@kindex load @var{filename}
>> -@item load @var{filename}
>> +@kindex load @var{filename} @var{offset}
>> +@item load @var{filename} @var{offset}
>>  @anchor{load}
>>  Depending on what remote debugging facilities are configured into
>>  @value{GDBN}, the @code{load} command may be available.  Where it
>> exists, it
>> @@ -19611,6 +19611,10 @@ link the program; for other formats, like
>> a.out, the object file format
>>  specifies a fixed address.
>>  @c FIXME! This would be a good place for an xref to the GNU linker doc.
>>
>> +It is also possible to tell @value{GDBN} to load the symbol file at a
>> specific
>> +offset described by the optional argument @var{offset}.  When
>> @var{offset} is
>> +provided, @var{filename} must also be provided.
>> +
>>  Depending on the remote side capabilities, @value{GDBN} may be able to
>>  load programs into flash memory.
>>
>> diff --git a/gdb/symfile.c b/gdb/symfile.c
>> index 61b9e5b..6ffc86b 100644
>> --- a/gdb/symfile.c
>> +++ b/gdb/symfile.c
>> @@ -3952,7 +3952,10 @@ that lies within the boundaries of this symbol
>> file in memory."),
>>    c = add_cmd ("load", class_files, load_command, _("\
>>  Dynamically load FILE into the running program, and record its
>> symbols\n\
>>  for access from GDB.\n\
>> -A load OFFSET may also be given."), &cmdlist);
>> +An optional load OFFSET may also be given as a literal address.\n\
>> +When OFFSET is provided, FILE must also be provided.  FILE can be
>> provided\n\
>> +on its own.\n\
>> +Usage: load [FILE] [OFFSET]"), &cmdlist);
>>    set_cmd_completer (c, filename_completer);
>>
>>    add_prefix_cmd ("overlay", class_support, overlay_command,
>>
>
>
  
Luis Machado Feb. 7, 2017, 10:20 a.m. UTC | #3
Ping?

https://sourceware.org/ml/gdb-patches/2017-01/msg00116.html

On 01/06/2017 05:15 PM, Luis Machado wrote:
> Changes in v2:
>
> - Improved description of how FILE and OFFSET must be passed.
> - Complemented description in the GDB manual since it was missing the
>   description for OFFSET.
>
> This fairly obvious patch adds usage text to the load command's help text.
>
> Originally it did not have usage and mentioned things like FILE and OFFSET
> without explaining how those should be passed in the command.
>
> gdb/ChangeLog:
>
> 2017-01-06  Luis Machado  <lgustavo@codesourcery.com>
>
> 	* symfile (_initialize_symfile): Add usage text to the load command's
> 	help text.
>
> gdb/doc/ChangeLog:
>
> 2017-01-06  Luis Machado  <lgustavo@codesourcery.com>
>
> 	* gdb.texinfo (_initialize_symfile): Document the optional offset
> 	argument.
  
Pedro Alves Feb. 7, 2017, 11:12 a.m. UTC | #4
I suspect that the reason these weren't originally mentioned
in the help was/is that the accepted arguments depend on target.

Back when we had support for a bunch of different remote
targets, we had more target_load implementations.
Nowadays, there's much fewer targets, though still a couple
load implemenations:

 $ grep "[.\->]to_load = " *
 remote.c:  remote_ops.to_load = remote_load;
 remote-sim.c:  gdbsim_ops.to_load = gdbsim_load;

remote_load calls generic_load, which accepts the offset.
gdbsim_load doesn't support an offset, but understands that
what comes after the filename is an offset.

(we could probably factor out more bits from those
two implementations to load_command.)

> -@kindex load @var{filename}
> -@item load @var{filename}
> +@kindex load @var{filename} @var{offset}
> +@item load @var{filename} @var{offset}
>  @anchor{load}
>  Depending on what remote debugging facilities are configured into
>  @value{GDBN}, the @code{load} command may be available.  Where it exists, it
> @@ -19611,6 +19611,10 @@ link the program; for other formats, like a.out, the object file format
>  specifies a fixed address.
>  @c FIXME! This would be a good place for an xref to the GNU linker doc.
>  
> +It is also possible to tell @value{GDBN} to load the symbol file at a specific

executable file, not symbol file.

> +offset described by the optional argument @var{offset}.  When @var{offset} is
> +provided, @var{filename} must also be provided.

LGTM with that, but Eli should review this.

Thanks,
Pedro Alves
  
Eli Zaretskii Feb. 7, 2017, 3:53 p.m. UTC | #5
> From: Luis Machado <lgustavo@codesourcery.com>
> Date: Tue, 7 Feb 2017 04:20:44 -0600
> 
> Ping?
> 
> https://sourceware.org/ml/gdb-patches/2017-01/msg00116.html

Sorry, the documentation changes are OK, except ...

> > gdb/doc/ChangeLog:
> >
> > 2017-01-06  Luis Machado  <lgustavo@codesourcery.com>
> >
> > 	* gdb.texinfo (_initialize_symfile): Document the optional offset
> > 	argument.

... this: the thing in the parentheses should be the name of the node
where you make the changes.

Thanks.
  
Luis Machado Feb. 13, 2017, 1:32 p.m. UTC | #6
On 02/07/2017 05:12 AM, Pedro Alves wrote:
> I suspect that the reason these weren't originally mentioned
> in the help was/is that the accepted arguments depend on target.
>
> Back when we had support for a bunch of different remote
> targets, we had more target_load implementations.
> Nowadays, there's much fewer targets, though still a couple
> load implemenations:
>
>  $ grep "[.\->]to_load = " *
>  remote.c:  remote_ops.to_load = remote_load;
>  remote-sim.c:  gdbsim_ops.to_load = gdbsim_load;
>
> remote_load calls generic_load, which accepts the offset.
> gdbsim_load doesn't support an offset, but understands that
> what comes after the filename is an offset.
>
> (we could probably factor out more bits from those
> two implementations to load_command.)
>
>> -@kindex load @var{filename}
>> -@item load @var{filename}
>> +@kindex load @var{filename} @var{offset}
>> +@item load @var{filename} @var{offset}
>>  @anchor{load}
>>  Depending on what remote debugging facilities are configured into
>>  @value{GDBN}, the @code{load} command may be available.  Where it exists, it
>> @@ -19611,6 +19611,10 @@ link the program; for other formats, like a.out, the object file format
>>  specifies a fixed address.
>>  @c FIXME! This would be a good place for an xref to the GNU linker doc.
>>
>> +It is also possible to tell @value{GDBN} to load the symbol file at a specific
>
> executable file, not symbol file.
>
>> +offset described by the optional argument @var{offset}.  When @var{offset} is
>> +provided, @var{filename} must also be provided.
>
> LGTM with that, but Eli should review this.
>
> Thanks,
> Pedro Alves
>
>

Thanks. I've addressed both the above and Eli's comments. Pushed as 
5cf30ebf64d3c6da961094c615a94d2f1682a478.
  
Eli Zaretskii Feb. 13, 2017, 2:57 p.m. UTC | #7
> From: Luis Machado <lgustavo@codesourcery.com>
> Date: Mon, 13 Feb 2017 07:32:09 -0600
> 
> > LGTM with that, but Eli should review this.

LGTM, thanks.
  

Patch

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index ccb95ad..d9c77f7 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -19591,8 +19591,8 @@  Show the current status of displaying communications between
 
 @table @code
 
-@kindex load @var{filename}
-@item load @var{filename}
+@kindex load @var{filename} @var{offset}
+@item load @var{filename} @var{offset}
 @anchor{load}
 Depending on what remote debugging facilities are configured into
 @value{GDBN}, the @code{load} command may be available.  Where it exists, it
@@ -19611,6 +19611,10 @@  link the program; for other formats, like a.out, the object file format
 specifies a fixed address.
 @c FIXME! This would be a good place for an xref to the GNU linker doc.
 
+It is also possible to tell @value{GDBN} to load the symbol file at a specific
+offset described by the optional argument @var{offset}.  When @var{offset} is
+provided, @var{filename} must also be provided.
+
 Depending on the remote side capabilities, @value{GDBN} may be able to
 load programs into flash memory.
 
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 61b9e5b..6ffc86b 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3952,7 +3952,10 @@  that lies within the boundaries of this symbol file in memory."),
   c = add_cmd ("load", class_files, load_command, _("\
 Dynamically load FILE into the running program, and record its symbols\n\
 for access from GDB.\n\
-A load OFFSET may also be given."), &cmdlist);
+An optional load OFFSET may also be given as a literal address.\n\
+When OFFSET is provided, FILE must also be provided.  FILE can be provided\n\
+on its own.\n\
+Usage: load [FILE] [OFFSET]"), &cmdlist);
   set_cmd_completer (c, filename_completer);
 
   add_prefix_cmd ("overlay", class_support, overlay_command,