Re: [PATCH 2/6 v2] gdb: Add command 'maint set/show debuginfod download-sections'

[Eli Zaretskii <eliz@gnu.org>]

> Cc: aburgess@redhat.com,
> 	Aaron Merey <amerey@redhat.com>
> Date: Wed, 31 May 2023 21:43:43 -0400
> From: Aaron Merey via Gdb-patches <gdb-patches@sourceware.org>
> 
>  gdb/debuginfod-support.c | 66 ++++++++++++++++++++++++++++++++++++++--
>  gdb/doc/gdb.texinfo      | 16 ++++++++--
>  2 files changed, 77 insertions(+), 5 deletions(-)

Thanks.

> @@ -568,8 +604,11 @@ _initialize_debuginfod ()
>  			_("Set whether to use debuginfod."),
>  			_("Show whether to use debuginfod."),
>  			_("\
> -When on, enable the use of debuginfod to download missing debug info and\n\
> -source files."),
> +When set to \"on\", enable the use of debuginfod to download missing\n\
> +debug info and source files. GDB may also download components of debug\n\
> +info instead of entire files. \"off\" disables the use of debuginfod.\n\

Our conventions are to leave two spaces between sentences, not one.

> +When set to \"ask\", a prompt may ask whether to enable or disable\n\
> +debuginfod." ),

"a prompt may ask" sounds awkward and somewhat mysterious.  How about

  When set to "ask", prompt whether to enable or disable [...]

instead?

> +@kindex maint set debuginfod
> +@kindex maint show debuginfod
> +@item maint set debuginfod download-sections
> +@itemx maint set debuginfod download-sections @r{[}on|off@r{]}
> +@itemx maint show debuginfod download-sections
> +Controls whether @value{GDBN} will attempt to download individual
> +ELF/DWARF sections from @code{debuginfod}.  If disabled, only
> +whole debug info files will be downloaded.  This could result
> +in @value{GDBN} downloading larger amounts of data.

I'd make the last sentence a part of the one before it, for clarity of
what "this" means.  Like this:

   If disabled, only whole debug info files will be downloaded; this
   could result in @value{GDBN} downloading larger amounts of data.

>  @cindex @value{GDBN} internal error
>  @cindex internal errors, control of @value{GDBN} behavior
>  @cindex demangler crashes
> @@ -49382,8 +49392,10 @@ regarding @code{debuginfod}.
>  @item set debuginfod enabled
>  @itemx set debuginfod enabled on
>  @cindex enable debuginfod
> -@value{GDBN} will attempt to query @code{debuginfod} servers when missing debug
> -info or source files.
> +@value{GDBN} will attempt to query @code{debuginfod} servers for missing debug
> +info or source files.

Why "attempt to query"? why not just "query"?  Or maybe "attempt to
download"?

> @value{GDBN} may attempt to download individual ELF/DWARF
> +sections such as @code{.gdb_index} to help reduce the total amount of data
> +downloaded from debuginfod servers.

Once again, I'd drop the "attempt" part.  More importantly, a
cross-reference to where "maint set debuginfod" is described is
missing here.  I would suggest

  @value{GDBN} may download individual ELF/DWARF sections such as
  @code{.gdb_index} to help reduce the total amount of data downloaded
  from debuginfod servers; this can be controlled by @w{@code{maint
  set debuginfod download-sections}} (@pxref{Maintenance Commands,
  maint set debuginfod download-sections}).

Reviewed-By: Eli Zaretskii <eliz@gnu.org>