[PATCH] Improve MI -dprintf-insert documentation

[PATCH] Improve MI -dprintf-insert documentation

[Tom Tromey]

I found the documentation for -dprintf-insert a bit unclear.  It
didn't mention the possibility of multiple arguments, and I also
noticed that it implied that the format parameter is optional, which
it is not.

While looking into this I also noticed a few comments in the
implementation that could also be improved.

Then, I noticed a repeated call to strlen in a loop condition, so I
fixed this up as well.
---
 gdb/doc/gdb.texinfo   |  9 +++++++--
 gdb/mi/mi-cmd-break.c | 10 +++++-----
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a179af77224..fc55c4e7b43 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -32328,10 +32328,15 @@ times="0"@}]@}
 @smallexample
  -dprintf-insert [ -t ] [ -f ] [ -d ] [ --qualified ]
     [ -c @var{condition} ] [--force-condition] [ -i @var{ignore-count} ]
-    [ -p @var{thread-id} ] [ @var{locspec} ] [ @var{format} ]
-    [ @var{argument} ]
+    [ -p @var{thread-id} ] [ @var{locspec} ] @var{format}
+    [ @var{argument}@dots{} ]
 @end smallexample
 
+@noindent
+Insert a new dynamic print breakpoint at the given location.
+@xref{Dynamic Printf}.  @var{format} is the format to use, and any
+remaining arguments are passed as expressions to substitute.
+
 @noindent
 If supplied, @var{locspec} and @code{--qualified} may be specified
 the same way as for the @code{-break-insert} command.
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 082c4da58a3..0777fcbd35e 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -93,8 +93,8 @@ setup_breakpoint_reporting (void)
 }
 
 
-/* Convert arguments in ARGV to the string in "format",argv,argv...
-   and return it.  */
+/* Convert arguments in ARGV to a string suitable for parsing by
+   dprintf like "FORMAT",ARG,ARG... and return it.  */
 
 static std::string
 mi_argv_to_format (const char *const *argv, int argc)
@@ -102,9 +102,9 @@ mi_argv_to_format (const char *const *argv, int argc)
   int i;
   std::string result;
 
-  /* Convert ARGV[OIND + 1] to format string and save to FORMAT.  */
+  /* Convert ARGV[0] to format string and save to FORMAT.  */
   result += '\"';
-  for (i = 0; i < strlen (argv[0]); i++)
+  for (i = 0; argv[0][i] != '\0'; i++)
     {
       switch (argv[0][i])
 	{
@@ -151,7 +151,7 @@ mi_argv_to_format (const char *const *argv, int argc)
     }
   result += '\"';
 
-  /* Apply other argv to FORMAT.  */
+  /* Append other arguments.  */
   for (i = 1; i < argc; i++)
     {
       result += ',';
-- 
2.40.0

Re: [PATCH] Improve MI -dprintf-insert documentation

[Eli Zaretskii]

> Cc: Tom Tromey <tromey@adacore.com>
> Date: Wed, 31 May 2023 08:39:32 -0600
> From: Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
> 
> I found the documentation for -dprintf-insert a bit unclear.  It
> didn't mention the possibility of multiple arguments, and I also
> noticed that it implied that the format parameter is optional, which
> it is not.
> 
> While looking into this I also noticed a few comments in the
> implementation that could also be improved.
> 
> Then, I noticed a repeated call to strlen in a loop condition, so I
> fixed this up as well.
> ---
>  gdb/doc/gdb.texinfo   |  9 +++++++--
>  gdb/mi/mi-cmd-break.c | 10 +++++-----
>  2 files changed, 12 insertions(+), 7 deletions(-)

OK for the documentation part, thanks.

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

Re: [PATCH] Improve MI -dprintf-insert documentation

[Andrew Burgess]

Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

> I found the documentation for -dprintf-insert a bit unclear.  It
> didn't mention the possibility of multiple arguments, and I also
> noticed that it implied that the format parameter is optional, which
> it is not.
>
> While looking into this I also noticed a few comments in the
> implementation that could also be improved.
>
> Then, I noticed a repeated call to strlen in a loop condition, so I
> fixed this up as well.

Looks good.

Reviewed-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


> ---
>  gdb/doc/gdb.texinfo   |  9 +++++++--
>  gdb/mi/mi-cmd-break.c | 10 +++++-----
>  2 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index a179af77224..fc55c4e7b43 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -32328,10 +32328,15 @@ times="0"@}]@}
>  @smallexample
>   -dprintf-insert [ -t ] [ -f ] [ -d ] [ --qualified ]
>      [ -c @var{condition} ] [--force-condition] [ -i @var{ignore-count} ]
> -    [ -p @var{thread-id} ] [ @var{locspec} ] [ @var{format} ]
> -    [ @var{argument} ]
> +    [ -p @var{thread-id} ] [ @var{locspec} ] @var{format}
> +    [ @var{argument}@dots{} ]
>  @end smallexample
>  
> +@noindent
> +Insert a new dynamic print breakpoint at the given location.
> +@xref{Dynamic Printf}.  @var{format} is the format to use, and any
> +remaining arguments are passed as expressions to substitute.
> +
>  @noindent
>  If supplied, @var{locspec} and @code{--qualified} may be specified
>  the same way as for the @code{-break-insert} command.
> diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
> index 082c4da58a3..0777fcbd35e 100644
> --- a/gdb/mi/mi-cmd-break.c
> +++ b/gdb/mi/mi-cmd-break.c
> @@ -93,8 +93,8 @@ setup_breakpoint_reporting (void)
>  }
>  
>  
> -/* Convert arguments in ARGV to the string in "format",argv,argv...
> -   and return it.  */
> +/* Convert arguments in ARGV to a string suitable for parsing by
> +   dprintf like "FORMAT",ARG,ARG... and return it.  */
>  
>  static std::string
>  mi_argv_to_format (const char *const *argv, int argc)
> @@ -102,9 +102,9 @@ mi_argv_to_format (const char *const *argv, int argc)
>    int i;
>    std::string result;
>  
> -  /* Convert ARGV[OIND + 1] to format string and save to FORMAT.  */
> +  /* Convert ARGV[0] to format string and save to FORMAT.  */
>    result += '\"';
> -  for (i = 0; i < strlen (argv[0]); i++)
> +  for (i = 0; argv[0][i] != '\0'; i++)
>      {
>        switch (argv[0][i])
>  	{
> @@ -151,7 +151,7 @@ mi_argv_to_format (const char *const *argv, int argc)
>      }
>    result += '\"';
>  
> -  /* Apply other argv to FORMAT.  */
> +  /* Append other arguments.  */
>    for (i = 1; i < argc; i++)
>      {
>        result += ',';
> -- 
> 2.40.0