public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2] gdb: split array and string limiting options
@ 2021-12-14 16:38 Maciej W. Rozycki
  2021-12-14 17:24 ` Eli Zaretskii
  2021-12-15 18:01 ` Andrew Burgess
  0 siblings, 2 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2021-12-14 16:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess, Simon Marchi

From: Andrew Burgess <andrew.burgess@embecosm.com>

This commit splits the set/show print elements options into two.  We 
retain set/show print elements for controlling how many elements of an 
array we print, but a new set/show print characters is added which is 
used for controlling how many characters of a string are printed.

The motivation behind this change is to allow users a finer level of 
control over how data is printed, reflecting that, although strings can 
be thought of as arrays of characters, users often want to treat these 
two things differently.

The GDB changes are all pretty straight forward, just changing 
references to the old 'print_max' to reference the new 'print_max_chars' 
setting.

Likewise, the documentation is just updated to reference the new
setting where appropriate.

In the testsuite, most of the changes are places where I've either 
changed to using 'set print characters' instead of 'set print elements', 
or I set both together as the subsequent tests were a mix of arrays and 
strings.

There's new tests for Ada and Pascal, as the string printing code for 
these languages is different than the generic string printing code used 
by other languages.  Modula2 also has different string printing code, 
but (a) this is similar to Pascal, and (b) there are no existing modula2 
tests written in Modula2, so I'm not sure how I'd even test the Modula2 
string printing.

There's also a few places where GDB used 'show print elements', the 
message printed from this command is now different, so required a few 
updates too.

Co-Authored-By: Maciej W. Rozycki <macro@embecosm.com>
---
Changes from v1:

- rename `print_smax' setting throughout to `print_max_chars', and
  likewise `show_print_smax' function to `show_print_max_chars',

- document the Python part in the manual,

- update comments for `print_max' and `print_max_chars' in
  `value_print_options',

- fix some typos.
---
 gdb/NEWS                                      |   18 +++++++
 gdb/ada-valprint.c                            |    4 -
 gdb/c-lang.c                                  |    4 -
 gdb/c-valprint.c                              |    4 -
 gdb/doc/gdb.texinfo                           |   36 +++++++++++---
 gdb/doc/python.texi                           |    5 ++
 gdb/language.h                                |    2 
 gdb/m2-lang.c                                 |    2 
 gdb/m2-valprint.c                             |    3 -
 gdb/p-lang.c                                  |    2 
 gdb/p-valprint.c                              |    7 +-
 gdb/printcmd.c                                |    8 +--
 gdb/python/py-value.c                         |    6 ++
 gdb/testsuite/gdb.ada/str_chars.exp           |   64 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.ada/str_chars/foo.adb       |   26 ++++++++++
 gdb/testsuite/gdb.base/default.exp            |    4 -
 gdb/testsuite/gdb.base/examine-backward.exp   |    6 ++
 gdb/testsuite/gdb.base/options.exp            |    1 
 gdb/testsuite/gdb.base/printcmds.exp          |   28 +++++------
 gdb/testsuite/gdb.base/wchar.exp              |    6 +-
 gdb/testsuite/gdb.base/with.exp               |    2 
 gdb/testsuite/gdb.guile/scm-pretty-print.exp  |   12 ++--
 gdb/testsuite/gdb.pascal/str-chars.exp        |   48 +++++++++++++++++++
 gdb/testsuite/gdb.pascal/str-chars.pas        |   28 +++++++++++
 gdb/testsuite/gdb.python/py-format-string.exp |   11 ++--
 gdb/testsuite/gdb.python/py-prettyprint.exp   |    8 ++-
 gdb/testsuite/lib/gdb.exp                     |    2 
 gdb/tracepoint.c                              |    4 -
 gdb/valprint.c                                |   64 ++++++++++++++++++--------
 gdb/valprint.h                                |   10 ++--
 30 files changed, 342 insertions(+), 83 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/str_chars.exp
 create mode 100644 gdb/testsuite/gdb.ada/str_chars/foo.adb
 create mode 100644 gdb/testsuite/gdb.pascal/str-chars.exp
 create mode 100644 gdb/testsuite/gdb.pascal/str-chars.pas

Index: src/gdb/NEWS
===================================================================
--- src.orig/gdb/NEWS
+++ src/gdb/NEWS
@@ -53,6 +53,24 @@ maint packet
   as escaped hex, e.g. \x?? where '??' is replaces with the value of
   the non-printable character.
 
+set print characters LIMIT
+show print characters
+print -characters LIMIT
+  This new setting is like 'set print elements', but controls how many
+  characters of a string are printed.  This functionality used to be
+  covered by 'set print elements', but is now a separate setting.
+  LIMIT can be set to 'unlimited' to print all characters of a string.
+  The default value for LIMIT is 200.
+
+* Changed commands
+
+set print elements LIMIT
+show print elements
+print -elements LIMIT
+  This setting no longer affects how many characters of a string are
+  printed, instead the new 'set print characters' setting should be
+  used.
+
 * Python API
 
   ** New function gdb.add_history(), which takes a gdb.Value object
Index: src/gdb/ada-valprint.c
===================================================================
--- src.orig/gdb/ada-valprint.c
+++ src/gdb/ada-valprint.c
@@ -459,7 +459,7 @@ printstr (struct ui_file *stream, struct
       return;
     }
 
-  for (i = 0; i < length && things_printed < options->print_max; i += 1)
+  for (i = 0; i < length && things_printed < options->print_max_chars; i += 1)
     {
       /* Position of the character we are examining
 	 to see whether it is repeated.  */
@@ -700,7 +700,7 @@ ada_val_print_string (struct type *type,
       /* Look for a NULL char.  */
       for (temp_len = 0;
 	   (temp_len < len
-	    && temp_len < options->print_max
+	    && temp_len < options->print_max_chars
 	    && char_at (valaddr + offset_aligned,
 			temp_len, eltlen, byte_order) != 0);
 	   temp_len += 1);
Index: src/gdb/c-lang.c
===================================================================
--- src.orig/gdb/c-lang.c
+++ src/gdb/c-lang.c
@@ -185,8 +185,8 @@ language_defn::printchar (int c, struct
 /* Print the character string STRING, printing at most LENGTH
    characters.  LENGTH is -1 if the string is nul terminated.  Each
    character is WIDTH bytes long.  Printing stops early if the number
-   hits print_max; repeat counts are printed as appropriate.  Print
-   ellipses at the end if we had to stop before printing LENGTH
+   hits print_max_chars; repeat counts are printed as appropriate.
+   Print ellipses at the end if we had to stop before printing LENGTH
    characters, or if FORCE_ELLIPSES.  */
 
 void
Index: src/gdb/c-valprint.c
===================================================================
--- src.orig/gdb/c-valprint.c
+++ src/gdb/c-valprint.c
@@ -271,7 +271,7 @@ c_value_print_array (struct value *val,
 
 	      for (temp_len = 0;
 		   (temp_len < len
-		    && temp_len < options->print_max
+		    && temp_len < options->print_max_chars
 		    && extract_unsigned_integer (valaddr + temp_len * eltlen,
 						 eltlen, byte_order) != 0);
 		   ++temp_len)
@@ -280,7 +280,7 @@ c_value_print_array (struct value *val,
 	      /* Force LA_PRINT_STRING to print ellipses if
 		 we've printed the maximum characters and
 		 the next character is not \000.  */
-	      if (temp_len == options->print_max && temp_len < len)
+	      if (temp_len == options->print_max_chars && temp_len < len)
 		{
 		  ULONGEST ival
 		    = extract_unsigned_integer (valaddr + temp_len * eltlen,
Index: src/gdb/doc/gdb.texinfo
===================================================================
--- src.orig/gdb/doc/gdb.texinfo
+++ src/gdb/doc/gdb.texinfo
@@ -9996,10 +9996,15 @@ Related setting: @ref{set print array}.
 Set printing of array indexes.
 Related setting: @ref{set print array-indexes}.
 
+@item -characters @var{number-of-characters}|@code{unlimited}
+Set limit on string characters to print.  The value @code{unlimited}
+causes there to be no limit.  Related setting: @ref{set print
+characters}.
+
 @item -elements @var{number-of-elements}|@code{unlimited}
-Set limit on string chars or array elements to print.  The value
-@code{unlimited} causes there to be no limit.  Related setting:
-@ref{set print elements}.
+Set limit on array elements to print.  The value @code{unlimited}
+causes there to be no limit.  Related setting: @ref{set print
+elements}.
 
 @item -max-depth @var{depth}|@code{unlimited}
 Set the threshold after which nested structures are replaced with
@@ -11367,6 +11372,23 @@ Stop printing element indexes when displ
 Show whether the index of each element is printed when displaying
 arrays.
 
+@anchor{set print characters}
+@item set print characters @var{number-of-characters}
+@itemx set print characters unlimited
+@cindex number of string characters to print
+@cindex limit on number of printed string characters
+Set a limit on how many characters of a string @value{GDBN} will
+print.  If @value{GDBN} is printing a large string, it stops printing
+after it has printed the number of characters set by the @code{set
+print characters} command.
+When @value{GDBN} starts, this limit is set to 200.
+Setting @var{number-of-characters} to @code{unlimited} or zero means
+that the number of characters to print is unlimited.
+
+@item show print characters
+Display the number of characters of a large string that @value{GDBN}
+will print.
+
 @anchor{set print elements}
 @item set print elements @var{number-of-elements}
 @itemx set print elements unlimited
@@ -11375,13 +11397,13 @@ arrays.
 Set a limit on how many elements of an array @value{GDBN} will print.
 If @value{GDBN} is printing a large array, it stops printing after it has
 printed the number of elements set by the @code{set print elements} command.
-This limit also applies to the display of strings.
 When @value{GDBN} starts, this limit is set to 200.
 Setting @var{number-of-elements} to @code{unlimited} or zero means
 that the number of elements to print is unlimited.
 
 @item show print elements
-Display the number of elements of a large array that @value{GDBN} will print.
+Display the number of elements of a large array that @value{GDBN} will
+print.
 
 @anchor{set print frame-arguments}
 @item set print frame-arguments @var{value}
@@ -14878,7 +14900,7 @@ The optional @var{mods} changes the usua
 @code{s} requests that pointers to chars be handled as strings, in
 particular collecting the contents of the memory being pointed at, up
 to the first zero.  The upper bound is by default the value of the
-@code{print elements} variable; if @code{s} is followed by a decimal
+@code{print characters} variable; if @code{s} is followed by a decimal
 number, that is the upper bound instead.  So for instance
 @samp{collect/s25 mystr} collects as many as 25 characters at
 @samp{mystr}.
@@ -27925,7 +27947,7 @@ of a command.
 (gdb) alias -a show print elms = show print elements
 (gdb) set p elms 20
 (gdb) show p elms
-Limit on string chars or array elements to print is 200.
+Limit on array elements to print is 200.
 @end smallexample
 
 Note that if you are defining an alias of a @samp{set} command,
Index: src/gdb/doc/python.texi
===================================================================
--- src.orig/gdb/doc/python.texi
+++ src/gdb/doc/python.texi
@@ -1017,6 +1017,11 @@ the @emph{declared} type should be used.
 representation of a C@t{++} object, @code{False} if they shouldn't (see
 @code{set print static-members} in @ref{Print Settings}).
 
+@item max_characters
+Number of string characters to print, or @code{0} to print an unlimited
+number of characters (see @code{set print characters} in @ref{Print
+Settings}).
+
 @item max_elements
 Number of array elements to print, or @code{0} to print an unlimited
 number of elements (see @code{set print elements} in @ref{Print
Index: src/gdb/language.h
===================================================================
--- src.orig/gdb/language.h
+++ src/gdb/language.h
@@ -534,7 +534,7 @@ struct language_defn
 			  struct ui_file * stream) const;
 
 /* Print the character string STRING, printing at most LENGTH characters.
-   Printing stops early if the number hits print_max; repeat counts
+   Printing stops early if the number hits print_max_chars; repeat counts
    are printed as appropriate.  Print ellipses at the end if we
    had to stop before printing LENGTH characters, or if FORCE_ELLIPSES.  */
 
Index: src/gdb/m2-lang.c
===================================================================
--- src.orig/gdb/m2-lang.c
+++ src/gdb/m2-lang.c
@@ -169,7 +169,7 @@ m2_language::printstr (struct ui_file *s
       return;
     }
 
-  for (i = 0; i < length && things_printed < options->print_max; ++i)
+  for (i = 0; i < length && things_printed < options->print_max_chars; ++i)
     {
       /* Position of the character we are examining
 	 to see whether it is repeated.  */
Index: src/gdb/m2-valprint.c
===================================================================
--- src.orig/gdb/m2-valprint.c
+++ src/gdb/m2-valprint.c
@@ -332,7 +332,8 @@ m2_language::value_print_inner (struct v
 		  /* Look for a NULL char.  */
 		  for (temp_len = 0;
 		       (valaddr[temp_len]
-			&& temp_len < len && temp_len < options->print_max);
+			&& temp_len < len
+			&& temp_len < options->print_max_chars);
 		       temp_len++);
 		  len = temp_len;
 		}
Index: src/gdb/p-lang.c
===================================================================
--- src.orig/gdb/p-lang.c
+++ src/gdb/p-lang.c
@@ -253,7 +253,7 @@ pascal_language::printstr (struct ui_fil
       return;
     }
 
-  for (i = 0; i < length && things_printed < options->print_max; ++i)
+  for (i = 0; i < length && things_printed < options->print_max_chars; ++i)
     {
       /* Position of the character we are examining
 	 to see whether it is repeated.  */
Index: src/gdb/p-valprint.c
===================================================================
--- src.orig/gdb/p-valprint.c
+++ src/gdb/p-valprint.c
@@ -109,9 +109,10 @@ pascal_language::value_print_inner (stru
 
 		    /* Look for a NULL char.  */
 		    for (temp_len = 0;
-			 extract_unsigned_integer (valaddr + temp_len * eltlen,
-						   eltlen, byte_order)
-			   && temp_len < len && temp_len < options->print_max;
+			 (extract_unsigned_integer
+			    (valaddr + temp_len * eltlen, eltlen, byte_order)
+			  && temp_len < len
+			  && temp_len < options->print_max_chars);
 			 temp_len++);
 		    len = temp_len;
 		  }
Index: src/gdb/printcmd.c
===================================================================
--- src.orig/gdb/printcmd.c
+++ src/gdb/printcmd.c
@@ -968,11 +968,11 @@ find_string_backward (struct gdbarch *gd
 	  int offset = (chars_to_read - i - 1) * char_size;
 
 	  if (integer_is_zero (&buffer[offset], char_size)
-	      || chars_counted == options->print_max)
+	      || chars_counted == options->print_max_chars)
 	    {
-	      /* Found '\0' or reached print_max.  As OFFSET is the offset to
-		 '\0', we add CHAR_SIZE to return the start address of
-		 a string.  */
+	      /* Found '\0' or reached print_max_chars.  As OFFSET
+		 is the offset to '\0', we add CHAR_SIZE to return
+		 the start address of a string.  */
 	      --count;
 	      string_start_addr = addr + offset + char_size;
 	      chars_counted = 0;
Index: src/gdb/python/py-value.c
===================================================================
--- src.orig/gdb/python/py-value.c
+++ src/gdb/python/py-value.c
@@ -623,6 +623,7 @@ valpy_format_string (PyObject *self, PyO
       "actual_objects",		/* See set print object on|off.  */
       "static_members",		/* See set print static-members on|off.  */
       /* C non-bool options.  */
+      "max_characters", 	/* See set print characters N.  */
       "max_elements", 		/* See set print elements N.  */
       "max_depth",		/* See set print max-depth N.  */
       "repeat_threshold",	/* See set print repeats.  */
@@ -668,7 +669,7 @@ valpy_format_string (PyObject *self, PyO
   char *format = NULL;
   if (!gdb_PyArg_ParseTupleAndKeywords (args,
 					kw,
-					"|O!O!O!O!O!O!O!O!O!O!IIIs",
+					"|O!O!O!O!O!O!O!O!O!O!IIIIs",
 					keywords,
 					&PyBool_Type, &raw_obj,
 					&PyBool_Type, &pretty_arrays_obj,
@@ -680,6 +681,7 @@ valpy_format_string (PyObject *self, PyO
 					&PyBool_Type, &deref_refs_obj,
 					&PyBool_Type, &actual_objects_obj,
 					&PyBool_Type, &static_members_obj,
+					&opts.print_max_chars,
 					&opts.print_max,
 					&opts.max_depth,
 					&opts.repeat_count_threshold,
@@ -713,6 +715,8 @@ valpy_format_string (PyObject *self, PyO
      unlimited, and 0 is a valid choice.  */
   if (opts.print_max == 0)
     opts.print_max = UINT_MAX;
+  if (opts.print_max_chars == 0)
+    opts.print_max_chars = UINT_MAX;
   if (opts.repeat_count_threshold == 0)
     opts.repeat_count_threshold = UINT_MAX;
 
Index: src/gdb/testsuite/gdb.ada/str_chars.exp
===================================================================
--- /dev/null
+++ src/gdb/testsuite/gdb.ada/str_chars.exp
@@ -0,0 +1,64 @@
+# Copyright 2021 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test GDB's 'set print characters' setting works for Ada strings.
+
+load_lib "ada.exp"
+
+if { [skip_ada_tests] } { return -1 }
+
+standard_ada_testfile foo
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug ]] != "" } {
+  return -1
+}
+
+clean_restart ${testfile}
+
+set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb]
+if ![runto "foo.adb:$bp_location" ] then {
+  return -1
+}
+
+gdb_test "print Arg" \
+    " = \"abcdefghijklmnopqrstuvwxyz\"" \
+    "print with default settings"
+
+gdb_test_no_output "set print characters 26"
+gdb_test "print Arg" \
+    " = \"abcdefghijklmnopqrstuvwxyz\"" \
+    "print with character limit of 26"
+
+gdb_test "print -characters 11 -- Arg" \
+    " = \"abcdefghijk\"\\.\\.\\." \
+    "print with character limit of 11"
+
+gdb_test_no_output "set print characters 10"
+gdb_test "print Arg" \
+    " = \"abcdefghij\"\\.\\.\\." \
+    "print with character limit of 10"
+
+gdb_test_no_output "set print characters unlimited"
+gdb_test "print Arg" \
+    " = \"abcdefghijklmnopqrstuvwxyz\"" \
+    "print with unlimited character limit"
+
+# The 'set print elements' used to control printing of characters in a
+# string, before we created 'set print characters'.  This test ensures
+# that 'set print elements' doens't effect string printing any more.
+gdb_test_no_output "set print elements 10"
+gdb_test "print Arg" \
+    " = \"abcdefghijklmnopqrstuvwxyz\"" \
+    "print with unlimited character limit, but lower element limit"
Index: src/gdb/testsuite/gdb.ada/str_chars/foo.adb
===================================================================
--- /dev/null
+++ src/gdb/testsuite/gdb.ada/str_chars/foo.adb
@@ -0,0 +1,26 @@
+--  Copyright 2021 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+procedure Foo is
+
+   procedure Blah (Arg : String) is
+   begin
+     null; -- STOP
+   end;
+
+begin
+
+   Blah ("abcdefghijklmnopqrstuvwxyz");
+end Foo;
Index: src/gdb/testsuite/gdb.base/default.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/default.exp
+++ src/gdb/testsuite/gdb.base/default.exp
@@ -667,7 +667,7 @@ gdb_test "show print asm-demangle" "Dema
 #test show print demangle
 gdb_test "show print demangle" "Demangling of encoded C\[+\]+/ObjC names when displaying symbols is on."
 #test show print elements
-gdb_test "show print elements" "Limit on string chars or array elements to print is 200."
+gdb_test "show print elements" "Limit on array elements to print is 200."
 #test show print object
 gdb_test "show print object" "Printing of object's derived type based on vtable info is on."
 #test show print pretty
@@ -702,7 +702,7 @@ gdb_test "show write" "Writing into exec
 set show_confirm_seen 0
 set show_prompt_seen 0
 gdb_test_multiple "show" "show" {
-    -re "confirm:  *Whether to confirm potentially dangerous operations is on.(\[^\r\n\]*\[\r\n\])+history filename:  *The filename in which to record the command history is (\[^\r\n\]*\[\r\n\])+history save:  *Saving of the history record on exit is on.(\[^\r\n\]*\[\r\n\])+history size:  *The size of the command history is(\[^\r\n\]*\[\r\n\])+listsize:  *Number of source lines gdb will list by default is 10(\[^\r\n]*\[\r\n\])+print elements:  *Limit on string chars or array elements to print is 200." {
+    -re "confirm:  *Whether to confirm potentially dangerous operations is on.(\[^\r\n\]*\[\r\n\])+history filename:  *The filename in which to record the command history is (\[^\r\n\]*\[\r\n\])+history save:  *Saving of the history record on exit is on.(\[^\r\n\]*\[\r\n\])+history size:  *The size of the command history is(\[^\r\n\]*\[\r\n\])+listsize:  *Number of source lines gdb will list by default is 10(\[^\r\n]*\[\r\n\])+print elements:  *Limit on array elements to print is 200." {
 	verbose "Confirm dislayed"
 	set show_confirm_seen 1
 	exp_continue
Index: src/gdb/testsuite/gdb.base/examine-backward.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/examine-backward.exp
+++ src/gdb/testsuite/gdb.base/examine-backward.exp
@@ -56,6 +56,7 @@ with_test_prefix "memory page boundary"
     set boundary [get_first_mapped_address]
     if {![is_address_zero_readable] && $boundary != 0} {
         gdb_test_no_output "set print elements 0"
+        gdb_test_no_output "set print characters 0"
         gdb_test_sequence "x/3s ${boundary}" "take 3 strings forward" {
             "0x"
             "0x"
@@ -145,6 +146,7 @@ gdb_test_no_output "set charset ASCII"
 
 with_test_prefix "char-width=1, print-max=20" {
     gdb_test_no_output "set print elements 20"
+    gdb_test_no_output "set print characters 20"
     gdb_test_sequence "x/6s &TestStrings" "take 6 strings forward" {
         "\"ABCDEFGHIJKLMNOPQRST\"\.\.\."
         "\"UVWXYZ\""
@@ -187,6 +189,7 @@ with_test_prefix "char-width=1, print-ma
 
 with_test_prefix "char-width=2, print-max=20" {
     gdb_test_no_output "set print elements 20"
+    gdb_test_no_output "set print characters 20"
     gdb_test_sequence "x/6sh &TestStringsH" "take 6 strings forward" {
         "u\"ABCDEFGHIJKLMNOPQRST\"\.\.\."
         "u\"UVWXYZ\""
@@ -229,6 +232,7 @@ with_test_prefix "char-width=2, print-ma
 
 with_test_prefix "char-width=4, print-max=20" {
     gdb_test_no_output "set print elements 20"
+    gdb_test_no_output "set print characters 20"
     gdb_test_sequence "x/6sw &TestStringsW" "take 6 strings forward" {
         "U\"ABCDEFGHIJKLMNOPQRST\"\.\.\."
         "U\"UVWXYZ\""
@@ -271,6 +275,7 @@ with_test_prefix "char-width=4, print-ma
 
 with_test_prefix "char-width=2, print-max=0" {
     gdb_test_no_output "set print elements 0"
+    gdb_test_no_output "set print characters 0"
     gdb_test_sequence "x/6sh &TestStringsH" "take 6 strings forward" {
         "u\"ABCDEFGHIJKLMNOPQRSTUVWXYZ\""
         "u\"\""
@@ -314,6 +319,7 @@ with_test_prefix "char-width=2, print-ma
 
 with_test_prefix "char-width=1, print-max=4" {
     gdb_test_no_output "set print elements 4"
+    gdb_test_no_output "set print characters 4"
     gdb_test_sequence "x/9s &TestStrings" "take 9 strings forward" {
         "\"ABCD\"\.\.\."
         "\"EFGH\"\.\.\."
Index: src/gdb/testsuite/gdb.base/options.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/options.exp
+++ src/gdb/testsuite/gdb.base/options.exp
@@ -165,6 +165,7 @@ proc_with_prefix test-print {{prefix ""}
 	"-address"
 	"-array"
 	"-array-indexes"
+	"-characters"
 	"-elements"
 	"-max-depth"
 	"-memory-tag-violations"
Index: src/gdb/testsuite/gdb.base/printcmds.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/printcmds.exp
+++ src/gdb/testsuite/gdb.base/printcmds.exp
@@ -438,7 +438,7 @@ proc test_print_repeats_10 {} {
     global gdb_prompt decimal
 
     for { set x 1 } { $x <= 16 } { incr x } {
-	gdb_test_no_output "set print elements $x" "elements $x repeats"
+	gdb_test_no_output "set print characters $x" "set print characters $x repeats"
 	for { set e 1 } { $e <= 16 } {incr e } {
 	    set v [expr $e - 1]
 	    set command "p &ctable2\[${v}*16\]"
@@ -478,7 +478,7 @@ proc test_print_repeats_10 {} {
 		set xstr "${xstr}\[.\]\[.\]\[.\]"
 	    }
 	    set string " = \[(\]unsigned char \[*\]\[)\] <ctable2(\\+$decimal)?> ${a}${xstr}"
-	    gdb_test "$command" "$string" "$command with print elements set to $x"
+	    gdb_test "$command" "$string" "$command with print characters set to $x"
 	}
     }
 }
@@ -503,26 +503,26 @@ proc test_print_strings {} {
 
     # Test that setting print elements unlimited doesn't completely suppress
     # printing; this was a bug in older gdb's.
-    gdb_test_no_output "set print elements 0"
+    gdb_test_no_output "set print characters 0"
     gdb_test "p teststring" \
-	" = (.unsigned char .. )?\"teststring contents\"" "p teststring with elements set to 0"
-    gdb_test_no_output "set print elements 1"
+	" = (.unsigned char .. )?\"teststring contents\"" "p teststring with characters set to 0"
+    gdb_test_no_output "set print characters 1"
     gdb_test "p teststring" \
-	" = (.unsigned char .. )?\"t\"\\.\\.\\." "p teststring with elements set to 1"
-    gdb_test_no_output "set print elements 5"
+	" = (.unsigned char .. )?\"t\"\\.\\.\\." "p teststring with characters set to 1"
+    gdb_test_no_output "set print characters 5"
     gdb_test "p teststring" \
-	" = (.unsigned char .. )?\"tests\"\\.\\.\\." "p teststring with elements set to 5"
-    gdb_test_no_output "set print elements 19"
+	" = (.unsigned char .. )?\"tests\"\\.\\.\\." "p teststring with characters set to 5"
+    gdb_test_no_output "set print characters 19"
     gdb_test "p teststring" \
-	" = (.unsigned char .. )?\"teststring contents\"" "p teststring with elements set to 19"
-    gdb_test_no_output "set print elements 20"
+	" = (.unsigned char .. )?\"teststring contents\"" "p teststring with characters set to 19"
+    gdb_test_no_output "set print characters 20"
     gdb_test "p teststring" \
-	" = (.unsigned char .. )?\"teststring contents\"" "p teststring with elements set to 20"
+	" = (.unsigned char .. )?\"teststring contents\"" "p teststring with characters set to 20"
 
     gdb_test "print teststring2" \
 	" = \\(charptr\\) \"more contents\""
 
-    gdb_test_no_output "set print elements 8"
+    gdb_test_no_output "set print characters 8"
 
     # Set the target-charset to ASCII, because the output varies from
     # different charset.
@@ -693,7 +693,7 @@ proc test_print_char_arrays {} {
 proc test_print_string_constants {} {
     global gdb_prompt
 
-    gdb_test_no_output "set print elements 50"
+    gdb_test_no_output "set print characters 50"
 
     if [target_info exists gdb,cannot_call_functions] {
 	unsupported "this target can not call functions"
Index: src/gdb/testsuite/gdb.base/wchar.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/wchar.exp
+++ src/gdb/testsuite/gdb.base/wchar.exp
@@ -62,13 +62,13 @@ gdb_test_no_output "set print null on"
 gdb_test "print repeat" "= L\"A\", '$cent' <repeats 21 times>, \"B\"" \
     "print repeat (print null on)"
 
-gdb_test_no_output "set print elements 3"
+gdb_test_no_output "set print characters 3"
 
 gdb_test "print repeat" "= L\"A$cent$cent\"\.\.\." \
-    "print repeat (print elements 3)"
+    "print repeat (print characters 3)"
 
 gdb_test "print repeat_p" "= $hex L\"A$cent$cent\"\.\.\." \
-    "print repeat_p (print elements 3)"
+    "print repeat_p (print characters 3)"
 
 # From PR cli/14977, but here because it requires wchar_t.
 gdb_test "printf \"%ls\\n\", 0" "\\(null\\)"
Index: src/gdb/testsuite/gdb.base/with.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/with.exp
+++ src/gdb/testsuite/gdb.base/with.exp
@@ -252,7 +252,7 @@ with_test_prefix "errors" {
     gdb_test "with print elements 1 -- unknowncommand" \
 	"Undefined command: \"unknowncommand\"\\.  Try \"help\"\\."
     gdb_test "show print elements" \
-	"Limit on string chars or array elements to print is 200\\."
+	"Limit on array elements to print is 200\\."
 }
 
 # Check completion.
Index: src/gdb/testsuite/gdb.guile/scm-pretty-print.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.guile/scm-pretty-print.exp
+++ src/gdb/testsuite/gdb.guile/scm-pretty-print.exp
@@ -76,13 +76,15 @@ proc run_lang_tests {exefile lang} {
 		" = \{.*<Vbase1> = pp class name: Vbase1.*<Vbase2> = \{.*<VirtualTest> = pp value variable is: 1,.*members of Vbase2:.*_vptr.Vbase2 = $hex.*<Vbase3> = \{.*members of Vbase3.*members of Derived:.*value = 2.*"
 	    gdb_test "print ns " "\"embedded\\\\000null\\\\000string\"" \
 		"print ns with 200 elements"
-	    gdb_test_no_output "set print elements 3" ""
+	    gdb_test_no_output "set print characters 3" ""
 	    gdb_test "print ns" "emb\.\.\.." \
-		"print ns with 3 elements"
-	    gdb_test_no_output "set print elements 10" ""
+		"print ns with 3 characters"
+	    gdb_test "print -characters 4 -- ns" "embe\.\.\.." \
+		"print ns with 4 characters"
+	    gdb_test_no_output "set print characters 10" ""
 	    gdb_test "print ns" "embedded\\\\000n\.\.\.." \
-		"print ns with 10 elements"
-	    gdb_test_no_output "set print elements 200" ""
+		"print ns with 10 characters"
+	    gdb_test_no_output "set print characters 200" ""
 	}
 
 	if { ![is_address_zero_readable] } {
Index: src/gdb/testsuite/gdb.pascal/str-chars.exp
===================================================================
--- /dev/null
+++ src/gdb/testsuite/gdb.pascal/str-chars.exp
@@ -0,0 +1,48 @@
+# Copyright 2021 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "pascal.exp"
+
+standard_testfile .pas
+
+if {[gdb_compile_pascal "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug ]] != "" } {
+  return -1
+}
+
+clean_restart ${testfile}
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "set breakpoint 1 here"]
+
+# Verify that "start" lands inside the right procedure.
+if { [gdb_start_cmd] < 0 } {
+    untested start
+    return -1
+}
+
+gdb_continue_to_breakpoint "continue to breakpoint"
+
+gdb_test "print message" " = 'abcdefghijklmnopqrstuvwxyz'" \
+    "print message with the default settings"
+
+gdb_test_no_output "set print elements 10"
+gdb_test "print message" " = 'abcdefghijklmnopqrstuvwxyz'" \
+    "print message with 'print elements' set to 10"
+
+gdb_test_no_output "set print characters 10"
+gdb_test "print message" " = 'abcdefghij'\\.\\.\\." \
+    "print message with 'print characters' set to 10"
+
+gdb_test_no_output "set print characters unlimited"
+gdb_test "print message" " = 'abcdefghijklmnopqrstuvwxyz'" \
+    "print message with 'print characters' set to unlimited"
Index: src/gdb/testsuite/gdb.pascal/str-chars.pas
===================================================================
--- /dev/null
+++ src/gdb/testsuite/gdb.pascal/str-chars.pas
@@ -0,0 +1,28 @@
+{
+ Copyright 2021 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program.  If not, see <http://www.gnu.org/licenses/>.
+}
+
+program str_char;
+
+var
+   message : string;
+
+begin
+
+   message := 'abcdefghijklmnopqrstuvwxyz';
+   writeln (message)	{ set breakpoint 1 here }
+
+end.
Index: src/gdb/testsuite/gdb.python/py-format-string.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.python/py-format-string.exp
+++ src/gdb/testsuite/gdb.python/py-format-string.exp
@@ -632,7 +632,7 @@ proc_with_prefix test_max_elements {} {
 
   # 200 is the default maximum number of elements, so setting it should
   # not change the output.
-  set opts "max_elements=200"
+  set opts "max_elements=200, max_characters=200"
   with_test_prefix $opts {
     check_format_string "a_point_t" $opts
     check_format_string "a_point_t_pointer" $opts
@@ -653,7 +653,7 @@ proc_with_prefix test_max_elements {} {
     }
   }
 
-  set opts "max_elements=3"
+  set opts "max_elements=3, max_characters=3"
   with_test_prefix $opts {
     check_format_string "a_point_t" $opts
     check_format_string "a_point_t_pointer" $opts
@@ -683,7 +683,7 @@ proc_with_prefix test_max_elements {} {
 
   # Both 1,000 (we don't have that many elements) and 0 (unlimited) should
   # mean no truncation.
-  foreach opts { "max_elements=1000" "max_elements=0" } {
+  foreach opts { "max_elements=1000, max_characters=1000" "max_elements=0, max_characters=0" } {
     with_test_prefix $opts {
       check_format_string "a_point_t" $opts
       check_format_string "a_point_t_pointer" $opts
@@ -706,13 +706,16 @@ proc_with_prefix test_max_elements {} {
     }
   }
 
-  with_temp_option "set print elements 4" "set print elements 200" {
+  with_temp_option "set print characters 4" "set print characters 200" {
     check_format_string "a_string" "" \
       "${default_pointer_regexp} \"hell\"..."
     check_format_string "a_binary_string" "" \
       "${default_pointer_regexp} \"hell\"..."
     check_format_string "a_binary_string_array" "" \
       "\"hell\"..."
+  }
+
+  with_temp_option "set print elements 4" "set print elements 200" {
     check_format_string "an_array_with_repetition" "" \
       "\\{1, 3 <repeats 12 times>...\\}"
   }
Index: src/gdb/testsuite/gdb.python/py-prettyprint.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.python/py-prettyprint.exp
+++ src/gdb/testsuite/gdb.python/py-prettyprint.exp
@@ -80,13 +80,15 @@ proc run_lang_tests {exefile lang} {
 	    " = \{.*<Vbase1> = pp class name: Vbase1.*<Vbase2> = \{.*<VirtualTest> = pp value variable is: 1,.*members of Vbase2:.*_vptr.Vbase2 = $hex.*<Vbase3> = \{.*members of Vbase3.*members of Derived:.*value = 2.*"
 	gdb_test "print ns " "\"embedded\\\\000null\\\\000string\"" \
 	    "print ns with default element limit"
-	gdb_test_no_output "set print elements 3"
+	gdb_test_no_output "set print characters 3"
 	gdb_test "print ns" "emb\.\.\.." \
 	    "print ns with element limit of 3"
-	gdb_test_no_output "set print elements 10"
+	gdb_test "print -characters 4 -- ns" "embe\.\.\.." \
+	    "print ns with element limit of 4"
+	gdb_test_no_output "set print characters 10"
 	gdb_test "print ns" "embedded\\\\000n\.\.\.." \
 	    "print ns with element limit of 10"
-	gdb_test_no_output "set print elements 200"
+	gdb_test_no_output "set print characters 200"
     }
 
     if { ![is_address_zero_readable] } {
Index: src/gdb/testsuite/lib/gdb.exp
===================================================================
--- src.orig/gdb/testsuite/lib/gdb.exp
+++ src/gdb/testsuite/lib/gdb.exp
@@ -6443,7 +6443,7 @@ gdb_caching_proc gdb_has_argv0 {
 	set old_elements "200"
 	set test "show print elements"
 	gdb_test_multiple $test $test {
-	    -re "Limit on string chars or array elements to print is (\[^\r\n\]+)\\.\r\n$gdb_prompt $" {
+	    -re "Limit on array elements to print is (\[^\r\n\]+)\\.\r\n$gdb_prompt $" {
 		set old_elements $expect_out(1,string)
 	    }
 	}
Index: src/gdb/tracepoint.c
===================================================================
--- src.orig/gdb/tracepoint.c
+++ src/gdb/tracepoint.c
@@ -545,9 +545,9 @@ decode_agent_options (const char *exp, i
       if (target_supports_string_tracing ())
 	{
 	  /* Allow an optional decimal number giving an explicit maximum
-	     string length, defaulting it to the "print elements" value;
+	     string length, defaulting it to the "print characters" value;
 	     so "collect/s80 mystr" gets at most 80 bytes of string.  */
-	  *trace_string = opts.print_max;
+	  *trace_string = opts.print_max_chars;
 	  exp++;
 	  if (*exp >= '0' && *exp <= '9')
 	    *trace_string = atoi (exp);
Index: src/gdb/valprint.c
===================================================================
--- src.orig/gdb/valprint.c
+++ src/gdb/valprint.c
@@ -97,8 +97,11 @@ static void val_print_type_code_flags (s
 				       int embedded_offset,
 				       struct ui_file *stream);
 
-#define PRINT_MAX_DEFAULT 200	/* Start print_max off at this value.  */
-#define PRINT_MAX_DEPTH_DEFAULT 20	/* Start print_max_depth off at this value. */
+/* Start print_max and print_max_chars at this value.  */
+#define PRINT_MAX_DEFAULT 200
+
+/* Start print_max_depth at this value. */
+#define PRINT_MAX_DEPTH_DEFAULT 20
 
 struct value_print_options user_print_options =
 {
@@ -110,6 +113,7 @@ struct value_print_options user_print_op
   1,				/* addressprint */
   0,				/* objectprint */
   PRINT_MAX_DEFAULT,		/* print_max */
+  PRINT_MAX_DEFAULT,		/* print_max_chars */
   10,				/* repeat_count_threshold */
   0,				/* output_format */
   0,				/* format */
@@ -152,13 +156,25 @@ get_formatted_print_options (struct valu
   opts->format = format;
 }
 
+/* Implement 'show print elements'.  */
+
 static void
 show_print_max (struct ui_file *file, int from_tty,
 		struct cmd_list_element *c, const char *value)
 {
   fprintf_filtered (file,
-		    _("Limit on string chars or array "
-		      "elements to print is %s.\n"),
+		    _("Limit on array elements to print is %s.\n"),
+		    value);
+}
+
+/* Implement 'show print characters'.  */
+
+static void
+show_print_max_chars (struct ui_file *file, int from_tty,
+		      struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file,
+		    _("Limit on string characters to print is %s.\n"),
 		    value);
 }
 
@@ -2601,9 +2617,9 @@ print_converted_chars_to_obstack (struct
 /* Print the character string STRING, printing at most LENGTH
    characters.  LENGTH is -1 if the string is nul terminated.  TYPE is
    the type of each character.  OPTIONS holds the printing options;
-   printing stops early if the number hits print_max; repeat counts
-   are printed as appropriate.  Print ellipses at the end if we had to
-   stop before printing LENGTH characters, or if FORCE_ELLIPSES.
+   printing stops early if the number hits print_max_chars; repeat
+   counts are printed as appropriate.  Print ellipses at the end if we
+   had to stop before printing LENGTH characters, or if FORCE_ELLIPSES.
    QUOTE_CHAR is the character to print at each end of the string.  If
    C_STYLE_TERMINATOR is true, and the last character is 0, then it is
    omitted.  */
@@ -2657,7 +2673,7 @@ generic_printstr (struct ui_file *stream
   /* Convert characters until the string is over or the maximum
      number of printed characters has been reached.  */
   i = 0;
-  while (i < options->print_max)
+  while (i < options->print_max_chars)
     {
       int r;
 
@@ -2709,7 +2725,7 @@ generic_printstr (struct ui_file *stream
 /* Print a string from the inferior, starting at ADDR and printing up to LEN
    characters, of WIDTH bytes a piece, to STREAM.  If LEN is -1, printing
    stops at the first null byte, otherwise printing proceeds (including null
-   bytes) until either print_max or LEN characters have been printed,
+   bytes) until either print_max_chars or LEN characters have been printed,
    whichever is smaller.  ENCODING is the name of the string's
    encoding.  It can be NULL, in which case the target encoding is
    assumed.  */
@@ -2731,15 +2747,16 @@ val_print_string (struct type *elttype,
   int width = TYPE_LENGTH (elttype);
 
   /* First we need to figure out the limit on the number of characters we are
-     going to attempt to fetch and print.  This is actually pretty simple.  If
-     LEN >= zero, then the limit is the minimum of LEN and print_max.  If
-     LEN is -1, then the limit is print_max.  This is true regardless of
-     whether print_max is zero, UINT_MAX (unlimited), or something in between,
-     because finding the null byte (or available memory) is what actually
-     limits the fetch.  */
+     going to attempt to fetch and print.  This is actually pretty simple.
+     If LEN >= zero, then the limit is the minimum of LEN and print_max_chars.
+     If LEN is -1, then the limit is print_max_chars.  This is true regardless
+     of whether print_max_chars is zero, UINT_MAX (unlimited), or something in
+     between, because finding the null byte (or available memory) is what
+     actually limits the fetch.  */
 
-  fetchlimit = (len == -1 ? options->print_max : std::min ((unsigned) len,
-							   options->print_max));
+  fetchlimit = (len == -1
+		? options->print_max_chars
+		: std::min ((unsigned) len, options->print_max_chars));
 
   err = read_string (addr, len, width, fetchlimit, byte_order,
 		     &buffer, &bytes_read);
@@ -3020,8 +3037,17 @@ static const gdb::option::option_def val
     "elements",
     [] (value_print_options *opt) { return &opt->print_max; },
     show_print_max, /* show_cmd_cb */
-    N_("Set limit on string chars or array elements to print."),
-    N_("Show limit on string chars or array elements to print."),
+    N_("Set limit on array elements to print."),
+    N_("Show limit on array elements to print."),
+    N_("\"unlimited\" causes there to be no limit."),
+  },
+
+  uinteger_option_def {
+    "characters",
+    [] (value_print_options *opt) { return &opt->print_max_chars; },
+    show_print_max_chars, /* show_cmd_cb */
+    N_("Set limit on string chars to print."),
+    N_("Show limit on string chars to print."),
     N_("\"unlimited\" causes there to be no limit."),
   },
 
Index: src/gdb/valprint.h
===================================================================
--- src.orig/gdb/valprint.h
+++ src/gdb/valprint.h
@@ -48,12 +48,14 @@ struct value_print_options
      in its vtables.  */
   bool objectprint;
 
-  /* Maximum number of chars to print for a string pointer value or vector
-     contents, or UINT_MAX for no limit.  Note that "set print elements 0"
-     stores UINT_MAX in print_max, which displays in a show command as
-     "unlimited".  */
+  /* Maximum number of elements to print for vector contents, or UINT_MAX
+     for no limit.  Note that "set print elements 0" stores UINT_MAX in
+     print_max, which displays in a show command as "unlimited".  */
   unsigned int print_max;
 
+  /* Maximum number of string chars to print for a string pointer value.  */
+  unsigned int print_max_chars;
+
   /* Print repeat counts if there are more than this many repetitions
      of an element in an array.  */
   unsigned int repeat_count_threshold;

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gdb: split array and string limiting options
  2021-12-14 16:38 [PATCH v2] gdb: split array and string limiting options Maciej W. Rozycki
@ 2021-12-14 17:24 ` Eli Zaretskii
  2021-12-15 18:01 ` Andrew Burgess
  1 sibling, 0 replies; 11+ messages in thread
From: Eli Zaretskii @ 2021-12-14 17:24 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: gdb-patches, aburgess

> Date: Tue, 14 Dec 2021 16:38:43 +0000 (GMT)
> From: "Maciej W. Rozycki" <macro@embecosm.com>
> Cc: Andrew Burgess <aburgess@redhat.com>
> 
>  gdb/NEWS                                      |   18 +++++++
>  gdb/ada-valprint.c                            |    4 -
>  gdb/c-lang.c                                  |    4 -
>  gdb/c-valprint.c                              |    4 -
>  gdb/doc/gdb.texinfo                           |   36 +++++++++++---
>  gdb/doc/python.texi                           |    5 ++
>  gdb/language.h                                |    2 
>  gdb/m2-lang.c                                 |    2 
>  gdb/m2-valprint.c                             |    3 -
>  gdb/p-lang.c                                  |    2 
>  gdb/p-valprint.c                              |    7 +-
>  gdb/printcmd.c                                |    8 +--
>  gdb/python/py-value.c                         |    6 ++
>  gdb/testsuite/gdb.ada/str_chars.exp           |   64 ++++++++++++++++++++++++++
>  gdb/testsuite/gdb.ada/str_chars/foo.adb       |   26 ++++++++++
>  gdb/testsuite/gdb.base/default.exp            |    4 -
>  gdb/testsuite/gdb.base/examine-backward.exp   |    6 ++
>  gdb/testsuite/gdb.base/options.exp            |    1 
>  gdb/testsuite/gdb.base/printcmds.exp          |   28 +++++------
>  gdb/testsuite/gdb.base/wchar.exp              |    6 +-
>  gdb/testsuite/gdb.base/with.exp               |    2 
>  gdb/testsuite/gdb.guile/scm-pretty-print.exp  |   12 ++--
>  gdb/testsuite/gdb.pascal/str-chars.exp        |   48 +++++++++++++++++++
>  gdb/testsuite/gdb.pascal/str-chars.pas        |   28 +++++++++++
>  gdb/testsuite/gdb.python/py-format-string.exp |   11 ++--
>  gdb/testsuite/gdb.python/py-prettyprint.exp   |    8 ++-
>  gdb/testsuite/lib/gdb.exp                     |    2 
>  gdb/tracepoint.c                              |    4 -
>  gdb/valprint.c                                |   64 ++++++++++++++++++--------
>  gdb/valprint.h                                |   10 ++--
>  30 files changed, 342 insertions(+), 83 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.ada/str_chars.exp
>  create mode 100644 gdb/testsuite/gdb.ada/str_chars/foo.adb
>  create mode 100644 gdb/testsuite/gdb.pascal/str-chars.exp
>  create mode 100644 gdb/testsuite/gdb.pascal/str-chars.pas

Thanks, the documentation parts are OK.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gdb: split array and string limiting options
  2021-12-14 16:38 [PATCH v2] gdb: split array and string limiting options Maciej W. Rozycki
  2021-12-14 17:24 ` Eli Zaretskii
@ 2021-12-15 18:01 ` Andrew Burgess
  2022-01-04 23:15   ` Maciej W. Rozycki
  1 sibling, 1 reply; 11+ messages in thread
From: Andrew Burgess @ 2021-12-15 18:01 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: gdb-patches, Simon Marchi

* Maciej W. Rozycki <macro@embecosm.com> [2021-12-14 16:38:43 +0000]:

> From: Andrew Burgess <andrew.burgess@embecosm.com>
> 
> This commit splits the set/show print elements options into two.  We 
> retain set/show print elements for controlling how many elements of an 
> array we print, but a new set/show print characters is added which is 
> used for controlling how many characters of a string are printed.
> 
> The motivation behind this change is to allow users a finer level of 
> control over how data is printed, reflecting that, although strings can 
> be thought of as arrays of characters, users often want to treat these 
> two things differently.
> 
> The GDB changes are all pretty straight forward, just changing 
> references to the old 'print_max' to reference the new 'print_max_chars' 
> setting.
> 
> Likewise, the documentation is just updated to reference the new
> setting where appropriate.
> 
> In the testsuite, most of the changes are places where I've either 
> changed to using 'set print characters' instead of 'set print elements', 
> or I set both together as the subsequent tests were a mix of arrays and 
> strings.
> 
> There's new tests for Ada and Pascal, as the string printing code for 
> these languages is different than the generic string printing code used 
> by other languages.  Modula2 also has different string printing code, 
> but (a) this is similar to Pascal, and (b) there are no existing modula2 
> tests written in Modula2, so I'm not sure how I'd even test the Modula2 
> string printing.
> 
> There's also a few places where GDB used 'show print elements', the 
> message printed from this command is now different, so required a few 
> updates too.
> 
> Co-Authored-By: Maciej W. Rozycki <macro@embecosm.com>
> ---
> Changes from v1:
> 
> - rename `print_smax' setting throughout to `print_max_chars', and
>   likewise `show_print_smax' function to `show_print_max_chars',
> 
> - document the Python part in the manual,
> 
> - update comments for `print_max' and `print_max_chars' in
>   `value_print_options',
> 
> - fix some typos.

I have no objections to this version of the patch.  However, I don't
feel like I should approve this, given I wrote the original patch.
Hopefully, someone else can give this the once over.

I did see one small problem though...

> ---
>  gdb/NEWS                                      |   18 +++++++
>  gdb/ada-valprint.c                            |    4 -
>  gdb/c-lang.c                                  |    4 -
>  gdb/c-valprint.c                              |    4 -
>  gdb/doc/gdb.texinfo                           |   36 +++++++++++---
>  gdb/doc/python.texi                           |    5 ++
>  gdb/language.h                                |    2 
>  gdb/m2-lang.c                                 |    2 
>  gdb/m2-valprint.c                             |    3 -
>  gdb/p-lang.c                                  |    2 
>  gdb/p-valprint.c                              |    7 +-
>  gdb/printcmd.c                                |    8 +--
>  gdb/python/py-value.c                         |    6 ++
>  gdb/testsuite/gdb.ada/str_chars.exp           |   64 ++++++++++++++++++++++++++
>  gdb/testsuite/gdb.ada/str_chars/foo.adb       |   26 ++++++++++
>  gdb/testsuite/gdb.base/default.exp            |    4 -
>  gdb/testsuite/gdb.base/examine-backward.exp   |    6 ++
>  gdb/testsuite/gdb.base/options.exp            |    1 
>  gdb/testsuite/gdb.base/printcmds.exp          |   28 +++++------
>  gdb/testsuite/gdb.base/wchar.exp              |    6 +-
>  gdb/testsuite/gdb.base/with.exp               |    2 
>  gdb/testsuite/gdb.guile/scm-pretty-print.exp  |   12 ++--
>  gdb/testsuite/gdb.pascal/str-chars.exp        |   48 +++++++++++++++++++
>  gdb/testsuite/gdb.pascal/str-chars.pas        |   28 +++++++++++
>  gdb/testsuite/gdb.python/py-format-string.exp |   11 ++--
>  gdb/testsuite/gdb.python/py-prettyprint.exp   |    8 ++-
>  gdb/testsuite/lib/gdb.exp                     |    2 
>  gdb/tracepoint.c                              |    4 -
>  gdb/valprint.c                                |   64 ++++++++++++++++++--------
>  gdb/valprint.h                                |   10 ++--
>  30 files changed, 342 insertions(+), 83 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.ada/str_chars.exp
>  create mode 100644 gdb/testsuite/gdb.ada/str_chars/foo.adb
>  create mode 100644 gdb/testsuite/gdb.pascal/str-chars.exp
>  create mode 100644 gdb/testsuite/gdb.pascal/str-chars.pas
> 
> Index: src/gdb/NEWS
> ===================================================================
> --- src.orig/gdb/NEWS
> +++ src/gdb/NEWS
> @@ -53,6 +53,24 @@ maint packet
>    as escaped hex, e.g. \x?? where '??' is replaces with the value of
>    the non-printable character.
>  
> +set print characters LIMIT
> +show print characters
> +print -characters LIMIT
> +  This new setting is like 'set print elements', but controls how many
> +  characters of a string are printed.  This functionality used to be
> +  covered by 'set print elements', but is now a separate setting.
> +  LIMIT can be set to 'unlimited' to print all characters of a string.
> +  The default value for LIMIT is 200.
> +
> +* Changed commands
> +
> +set print elements LIMIT
> +show print elements
> +print -elements LIMIT
> +  This setting no longer affects how many characters of a string are
> +  printed, instead the new 'set print characters' setting should be
> +  used.
> +
>  * Python API

I suspect something went wrong with the rebase here.  The 'set print
characters' block should be under 'New commands', and the 'set print
elements' block should, indeed, be under 'Changed commands'.  But, I
think the 'Changed commands' you're adding here is now the second such
title for the current release of GDB.

Thanks,
Andrew

>  
>    ** New function gdb.add_history(), which takes a gdb.Value object
> Index: src/gdb/ada-valprint.c
> ===================================================================
> --- src.orig/gdb/ada-valprint.c
> +++ src/gdb/ada-valprint.c
> @@ -459,7 +459,7 @@ printstr (struct ui_file *stream, struct
>        return;
>      }
>  
> -  for (i = 0; i < length && things_printed < options->print_max; i += 1)
> +  for (i = 0; i < length && things_printed < options->print_max_chars; i += 1)
>      {
>        /* Position of the character we are examining
>  	 to see whether it is repeated.  */
> @@ -700,7 +700,7 @@ ada_val_print_string (struct type *type,
>        /* Look for a NULL char.  */
>        for (temp_len = 0;
>  	   (temp_len < len
> -	    && temp_len < options->print_max
> +	    && temp_len < options->print_max_chars
>  	    && char_at (valaddr + offset_aligned,
>  			temp_len, eltlen, byte_order) != 0);
>  	   temp_len += 1);
> Index: src/gdb/c-lang.c
> ===================================================================
> --- src.orig/gdb/c-lang.c
> +++ src/gdb/c-lang.c
> @@ -185,8 +185,8 @@ language_defn::printchar (int c, struct
>  /* Print the character string STRING, printing at most LENGTH
>     characters.  LENGTH is -1 if the string is nul terminated.  Each
>     character is WIDTH bytes long.  Printing stops early if the number
> -   hits print_max; repeat counts are printed as appropriate.  Print
> -   ellipses at the end if we had to stop before printing LENGTH
> +   hits print_max_chars; repeat counts are printed as appropriate.
> +   Print ellipses at the end if we had to stop before printing LENGTH
>     characters, or if FORCE_ELLIPSES.  */
>  
>  void
> Index: src/gdb/c-valprint.c
> ===================================================================
> --- src.orig/gdb/c-valprint.c
> +++ src/gdb/c-valprint.c
> @@ -271,7 +271,7 @@ c_value_print_array (struct value *val,
>  
>  	      for (temp_len = 0;
>  		   (temp_len < len
> -		    && temp_len < options->print_max
> +		    && temp_len < options->print_max_chars
>  		    && extract_unsigned_integer (valaddr + temp_len * eltlen,
>  						 eltlen, byte_order) != 0);
>  		   ++temp_len)
> @@ -280,7 +280,7 @@ c_value_print_array (struct value *val,
>  	      /* Force LA_PRINT_STRING to print ellipses if
>  		 we've printed the maximum characters and
>  		 the next character is not \000.  */
> -	      if (temp_len == options->print_max && temp_len < len)
> +	      if (temp_len == options->print_max_chars && temp_len < len)
>  		{
>  		  ULONGEST ival
>  		    = extract_unsigned_integer (valaddr + temp_len * eltlen,
> Index: src/gdb/doc/gdb.texinfo
> ===================================================================
> --- src.orig/gdb/doc/gdb.texinfo
> +++ src/gdb/doc/gdb.texinfo
> @@ -9996,10 +9996,15 @@ Related setting: @ref{set print array}.
>  Set printing of array indexes.
>  Related setting: @ref{set print array-indexes}.
>  
> +@item -characters @var{number-of-characters}|@code{unlimited}
> +Set limit on string characters to print.  The value @code{unlimited}
> +causes there to be no limit.  Related setting: @ref{set print
> +characters}.
> +
>  @item -elements @var{number-of-elements}|@code{unlimited}
> -Set limit on string chars or array elements to print.  The value
> -@code{unlimited} causes there to be no limit.  Related setting:
> -@ref{set print elements}.
> +Set limit on array elements to print.  The value @code{unlimited}
> +causes there to be no limit.  Related setting: @ref{set print
> +elements}.
>  
>  @item -max-depth @var{depth}|@code{unlimited}
>  Set the threshold after which nested structures are replaced with
> @@ -11367,6 +11372,23 @@ Stop printing element indexes when displ
>  Show whether the index of each element is printed when displaying
>  arrays.
>  
> +@anchor{set print characters}
> +@item set print characters @var{number-of-characters}
> +@itemx set print characters unlimited
> +@cindex number of string characters to print
> +@cindex limit on number of printed string characters
> +Set a limit on how many characters of a string @value{GDBN} will
> +print.  If @value{GDBN} is printing a large string, it stops printing
> +after it has printed the number of characters set by the @code{set
> +print characters} command.
> +When @value{GDBN} starts, this limit is set to 200.
> +Setting @var{number-of-characters} to @code{unlimited} or zero means
> +that the number of characters to print is unlimited.
> +
> +@item show print characters
> +Display the number of characters of a large string that @value{GDBN}
> +will print.
> +
>  @anchor{set print elements}
>  @item set print elements @var{number-of-elements}
>  @itemx set print elements unlimited
> @@ -11375,13 +11397,13 @@ arrays.
>  Set a limit on how many elements of an array @value{GDBN} will print.
>  If @value{GDBN} is printing a large array, it stops printing after it has
>  printed the number of elements set by the @code{set print elements} command.
> -This limit also applies to the display of strings.
>  When @value{GDBN} starts, this limit is set to 200.
>  Setting @var{number-of-elements} to @code{unlimited} or zero means
>  that the number of elements to print is unlimited.
>  
>  @item show print elements
> -Display the number of elements of a large array that @value{GDBN} will print.
> +Display the number of elements of a large array that @value{GDBN} will
> +print.
>  
>  @anchor{set print frame-arguments}
>  @item set print frame-arguments @var{value}
> @@ -14878,7 +14900,7 @@ The optional @var{mods} changes the usua
>  @code{s} requests that pointers to chars be handled as strings, in
>  particular collecting the contents of the memory being pointed at, up
>  to the first zero.  The upper bound is by default the value of the
> -@code{print elements} variable; if @code{s} is followed by a decimal
> +@code{print characters} variable; if @code{s} is followed by a decimal
>  number, that is the upper bound instead.  So for instance
>  @samp{collect/s25 mystr} collects as many as 25 characters at
>  @samp{mystr}.
> @@ -27925,7 +27947,7 @@ of a command.
>  (gdb) alias -a show print elms = show print elements
>  (gdb) set p elms 20
>  (gdb) show p elms
> -Limit on string chars or array elements to print is 200.
> +Limit on array elements to print is 200.
>  @end smallexample
>  
>  Note that if you are defining an alias of a @samp{set} command,
> Index: src/gdb/doc/python.texi
> ===================================================================
> --- src.orig/gdb/doc/python.texi
> +++ src/gdb/doc/python.texi
> @@ -1017,6 +1017,11 @@ the @emph{declared} type should be used.
>  representation of a C@t{++} object, @code{False} if they shouldn't (see
>  @code{set print static-members} in @ref{Print Settings}).
>  
> +@item max_characters
> +Number of string characters to print, or @code{0} to print an unlimited
> +number of characters (see @code{set print characters} in @ref{Print
> +Settings}).
> +
>  @item max_elements
>  Number of array elements to print, or @code{0} to print an unlimited
>  number of elements (see @code{set print elements} in @ref{Print
> Index: src/gdb/language.h
> ===================================================================
> --- src.orig/gdb/language.h
> +++ src/gdb/language.h
> @@ -534,7 +534,7 @@ struct language_defn
>  			  struct ui_file * stream) const;
>  
>  /* Print the character string STRING, printing at most LENGTH characters.
> -   Printing stops early if the number hits print_max; repeat counts
> +   Printing stops early if the number hits print_max_chars; repeat counts
>     are printed as appropriate.  Print ellipses at the end if we
>     had to stop before printing LENGTH characters, or if FORCE_ELLIPSES.  */
>  
> Index: src/gdb/m2-lang.c
> ===================================================================
> --- src.orig/gdb/m2-lang.c
> +++ src/gdb/m2-lang.c
> @@ -169,7 +169,7 @@ m2_language::printstr (struct ui_file *s
>        return;
>      }
>  
> -  for (i = 0; i < length && things_printed < options->print_max; ++i)
> +  for (i = 0; i < length && things_printed < options->print_max_chars; ++i)
>      {
>        /* Position of the character we are examining
>  	 to see whether it is repeated.  */
> Index: src/gdb/m2-valprint.c
> ===================================================================
> --- src.orig/gdb/m2-valprint.c
> +++ src/gdb/m2-valprint.c
> @@ -332,7 +332,8 @@ m2_language::value_print_inner (struct v
>  		  /* Look for a NULL char.  */
>  		  for (temp_len = 0;
>  		       (valaddr[temp_len]
> -			&& temp_len < len && temp_len < options->print_max);
> +			&& temp_len < len
> +			&& temp_len < options->print_max_chars);
>  		       temp_len++);
>  		  len = temp_len;
>  		}
> Index: src/gdb/p-lang.c
> ===================================================================
> --- src.orig/gdb/p-lang.c
> +++ src/gdb/p-lang.c
> @@ -253,7 +253,7 @@ pascal_language::printstr (struct ui_fil
>        return;
>      }
>  
> -  for (i = 0; i < length && things_printed < options->print_max; ++i)
> +  for (i = 0; i < length && things_printed < options->print_max_chars; ++i)
>      {
>        /* Position of the character we are examining
>  	 to see whether it is repeated.  */
> Index: src/gdb/p-valprint.c
> ===================================================================
> --- src.orig/gdb/p-valprint.c
> +++ src/gdb/p-valprint.c
> @@ -109,9 +109,10 @@ pascal_language::value_print_inner (stru
>  
>  		    /* Look for a NULL char.  */
>  		    for (temp_len = 0;
> -			 extract_unsigned_integer (valaddr + temp_len * eltlen,
> -						   eltlen, byte_order)
> -			   && temp_len < len && temp_len < options->print_max;
> +			 (extract_unsigned_integer
> +			    (valaddr + temp_len * eltlen, eltlen, byte_order)
> +			  && temp_len < len
> +			  && temp_len < options->print_max_chars);
>  			 temp_len++);
>  		    len = temp_len;
>  		  }
> Index: src/gdb/printcmd.c
> ===================================================================
> --- src.orig/gdb/printcmd.c
> +++ src/gdb/printcmd.c
> @@ -968,11 +968,11 @@ find_string_backward (struct gdbarch *gd
>  	  int offset = (chars_to_read - i - 1) * char_size;
>  
>  	  if (integer_is_zero (&buffer[offset], char_size)
> -	      || chars_counted == options->print_max)
> +	      || chars_counted == options->print_max_chars)
>  	    {
> -	      /* Found '\0' or reached print_max.  As OFFSET is the offset to
> -		 '\0', we add CHAR_SIZE to return the start address of
> -		 a string.  */
> +	      /* Found '\0' or reached print_max_chars.  As OFFSET
> +		 is the offset to '\0', we add CHAR_SIZE to return
> +		 the start address of a string.  */
>  	      --count;
>  	      string_start_addr = addr + offset + char_size;
>  	      chars_counted = 0;
> Index: src/gdb/python/py-value.c
> ===================================================================
> --- src.orig/gdb/python/py-value.c
> +++ src/gdb/python/py-value.c
> @@ -623,6 +623,7 @@ valpy_format_string (PyObject *self, PyO
>        "actual_objects",		/* See set print object on|off.  */
>        "static_members",		/* See set print static-members on|off.  */
>        /* C non-bool options.  */
> +      "max_characters", 	/* See set print characters N.  */
>        "max_elements", 		/* See set print elements N.  */
>        "max_depth",		/* See set print max-depth N.  */
>        "repeat_threshold",	/* See set print repeats.  */
> @@ -668,7 +669,7 @@ valpy_format_string (PyObject *self, PyO
>    char *format = NULL;
>    if (!gdb_PyArg_ParseTupleAndKeywords (args,
>  					kw,
> -					"|O!O!O!O!O!O!O!O!O!O!IIIs",
> +					"|O!O!O!O!O!O!O!O!O!O!IIIIs",
>  					keywords,
>  					&PyBool_Type, &raw_obj,
>  					&PyBool_Type, &pretty_arrays_obj,
> @@ -680,6 +681,7 @@ valpy_format_string (PyObject *self, PyO
>  					&PyBool_Type, &deref_refs_obj,
>  					&PyBool_Type, &actual_objects_obj,
>  					&PyBool_Type, &static_members_obj,
> +					&opts.print_max_chars,
>  					&opts.print_max,
>  					&opts.max_depth,
>  					&opts.repeat_count_threshold,
> @@ -713,6 +715,8 @@ valpy_format_string (PyObject *self, PyO
>       unlimited, and 0 is a valid choice.  */
>    if (opts.print_max == 0)
>      opts.print_max = UINT_MAX;
> +  if (opts.print_max_chars == 0)
> +    opts.print_max_chars = UINT_MAX;
>    if (opts.repeat_count_threshold == 0)
>      opts.repeat_count_threshold = UINT_MAX;
>  
> Index: src/gdb/testsuite/gdb.ada/str_chars.exp
> ===================================================================
> --- /dev/null
> +++ src/gdb/testsuite/gdb.ada/str_chars.exp
> @@ -0,0 +1,64 @@
> +# Copyright 2021 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test GDB's 'set print characters' setting works for Ada strings.
> +
> +load_lib "ada.exp"
> +
> +if { [skip_ada_tests] } { return -1 }
> +
> +standard_ada_testfile foo
> +
> +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug ]] != "" } {
> +  return -1
> +}
> +
> +clean_restart ${testfile}
> +
> +set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb]
> +if ![runto "foo.adb:$bp_location" ] then {
> +  return -1
> +}
> +
> +gdb_test "print Arg" \
> +    " = \"abcdefghijklmnopqrstuvwxyz\"" \
> +    "print with default settings"
> +
> +gdb_test_no_output "set print characters 26"
> +gdb_test "print Arg" \
> +    " = \"abcdefghijklmnopqrstuvwxyz\"" \
> +    "print with character limit of 26"
> +
> +gdb_test "print -characters 11 -- Arg" \
> +    " = \"abcdefghijk\"\\.\\.\\." \
> +    "print with character limit of 11"
> +
> +gdb_test_no_output "set print characters 10"
> +gdb_test "print Arg" \
> +    " = \"abcdefghij\"\\.\\.\\." \
> +    "print with character limit of 10"
> +
> +gdb_test_no_output "set print characters unlimited"
> +gdb_test "print Arg" \
> +    " = \"abcdefghijklmnopqrstuvwxyz\"" \
> +    "print with unlimited character limit"
> +
> +# The 'set print elements' used to control printing of characters in a
> +# string, before we created 'set print characters'.  This test ensures
> +# that 'set print elements' doens't effect string printing any more.
> +gdb_test_no_output "set print elements 10"
> +gdb_test "print Arg" \
> +    " = \"abcdefghijklmnopqrstuvwxyz\"" \
> +    "print with unlimited character limit, but lower element limit"
> Index: src/gdb/testsuite/gdb.ada/str_chars/foo.adb
> ===================================================================
> --- /dev/null
> +++ src/gdb/testsuite/gdb.ada/str_chars/foo.adb
> @@ -0,0 +1,26 @@
> +--  Copyright 2021 Free Software Foundation, Inc.
> +--
> +--  This program is free software; you can redistribute it and/or modify
> +--  it under the terms of the GNU General Public License as published by
> +--  the Free Software Foundation; either version 3 of the License, or
> +--  (at your option) any later version.
> +--
> +--  This program is distributed in the hope that it will be useful,
> +--  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +--  GNU General Public License for more details.
> +--
> +--  You should have received a copy of the GNU General Public License
> +--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +procedure Foo is
> +
> +   procedure Blah (Arg : String) is
> +   begin
> +     null; -- STOP
> +   end;
> +
> +begin
> +
> +   Blah ("abcdefghijklmnopqrstuvwxyz");
> +end Foo;
> Index: src/gdb/testsuite/gdb.base/default.exp
> ===================================================================
> --- src.orig/gdb/testsuite/gdb.base/default.exp
> +++ src/gdb/testsuite/gdb.base/default.exp
> @@ -667,7 +667,7 @@ gdb_test "show print asm-demangle" "Dema
>  #test show print demangle
>  gdb_test "show print demangle" "Demangling of encoded C\[+\]+/ObjC names when displaying symbols is on."
>  #test show print elements
> -gdb_test "show print elements" "Limit on string chars or array elements to print is 200."
> +gdb_test "show print elements" "Limit on array elements to print is 200."
>  #test show print object
>  gdb_test "show print object" "Printing of object's derived type based on vtable info is on."
>  #test show print pretty
> @@ -702,7 +702,7 @@ gdb_test "show write" "Writing into exec
>  set show_confirm_seen 0
>  set show_prompt_seen 0
>  gdb_test_multiple "show" "show" {
> -    -re "confirm:  *Whether to confirm potentially dangerous operations is on.(\[^\r\n\]*\[\r\n\])+history filename:  *The filename in which to record the command history is (\[^\r\n\]*\[\r\n\])+history save:  *Saving of the history record on exit is on.(\[^\r\n\]*\[\r\n\])+history size:  *The size of the command history is(\[^\r\n\]*\[\r\n\])+listsize:  *Number of source lines gdb will list by default is 10(\[^\r\n]*\[\r\n\])+print elements:  *Limit on string chars or array elements to print is 200." {
> +    -re "confirm:  *Whether to confirm potentially dangerous operations is on.(\[^\r\n\]*\[\r\n\])+history filename:  *The filename in which to record the command history is (\[^\r\n\]*\[\r\n\])+history save:  *Saving of the history record on exit is on.(\[^\r\n\]*\[\r\n\])+history size:  *The size of the command history is(\[^\r\n\]*\[\r\n\])+listsize:  *Number of source lines gdb will list by default is 10(\[^\r\n]*\[\r\n\])+print elements:  *Limit on array elements to print is 200." {
>  	verbose "Confirm dislayed"
>  	set show_confirm_seen 1
>  	exp_continue
> Index: src/gdb/testsuite/gdb.base/examine-backward.exp
> ===================================================================
> --- src.orig/gdb/testsuite/gdb.base/examine-backward.exp
> +++ src/gdb/testsuite/gdb.base/examine-backward.exp
> @@ -56,6 +56,7 @@ with_test_prefix "memory page boundary"
>      set boundary [get_first_mapped_address]
>      if {![is_address_zero_readable] && $boundary != 0} {
>          gdb_test_no_output "set print elements 0"
> +        gdb_test_no_output "set print characters 0"
>          gdb_test_sequence "x/3s ${boundary}" "take 3 strings forward" {
>              "0x"
>              "0x"
> @@ -145,6 +146,7 @@ gdb_test_no_output "set charset ASCII"
>  
>  with_test_prefix "char-width=1, print-max=20" {
>      gdb_test_no_output "set print elements 20"
> +    gdb_test_no_output "set print characters 20"
>      gdb_test_sequence "x/6s &TestStrings" "take 6 strings forward" {
>          "\"ABCDEFGHIJKLMNOPQRST\"\.\.\."
>          "\"UVWXYZ\""
> @@ -187,6 +189,7 @@ with_test_prefix "char-width=1, print-ma
>  
>  with_test_prefix "char-width=2, print-max=20" {
>      gdb_test_no_output "set print elements 20"
> +    gdb_test_no_output "set print characters 20"
>      gdb_test_sequence "x/6sh &TestStringsH" "take 6 strings forward" {
>          "u\"ABCDEFGHIJKLMNOPQRST\"\.\.\."
>          "u\"UVWXYZ\""
> @@ -229,6 +232,7 @@ with_test_prefix "char-width=2, print-ma
>  
>  with_test_prefix "char-width=4, print-max=20" {
>      gdb_test_no_output "set print elements 20"
> +    gdb_test_no_output "set print characters 20"
>      gdb_test_sequence "x/6sw &TestStringsW" "take 6 strings forward" {
>          "U\"ABCDEFGHIJKLMNOPQRST\"\.\.\."
>          "U\"UVWXYZ\""
> @@ -271,6 +275,7 @@ with_test_prefix "char-width=4, print-ma
>  
>  with_test_prefix "char-width=2, print-max=0" {
>      gdb_test_no_output "set print elements 0"
> +    gdb_test_no_output "set print characters 0"
>      gdb_test_sequence "x/6sh &TestStringsH" "take 6 strings forward" {
>          "u\"ABCDEFGHIJKLMNOPQRSTUVWXYZ\""
>          "u\"\""
> @@ -314,6 +319,7 @@ with_test_prefix "char-width=2, print-ma
>  
>  with_test_prefix "char-width=1, print-max=4" {
>      gdb_test_no_output "set print elements 4"
> +    gdb_test_no_output "set print characters 4"
>      gdb_test_sequence "x/9s &TestStrings" "take 9 strings forward" {
>          "\"ABCD\"\.\.\."
>          "\"EFGH\"\.\.\."
> Index: src/gdb/testsuite/gdb.base/options.exp
> ===================================================================
> --- src.orig/gdb/testsuite/gdb.base/options.exp
> +++ src/gdb/testsuite/gdb.base/options.exp
> @@ -165,6 +165,7 @@ proc_with_prefix test-print {{prefix ""}
>  	"-address"
>  	"-array"
>  	"-array-indexes"
> +	"-characters"
>  	"-elements"
>  	"-max-depth"
>  	"-memory-tag-violations"
> Index: src/gdb/testsuite/gdb.base/printcmds.exp
> ===================================================================
> --- src.orig/gdb/testsuite/gdb.base/printcmds.exp
> +++ src/gdb/testsuite/gdb.base/printcmds.exp
> @@ -438,7 +438,7 @@ proc test_print_repeats_10 {} {
>      global gdb_prompt decimal
>  
>      for { set x 1 } { $x <= 16 } { incr x } {
> -	gdb_test_no_output "set print elements $x" "elements $x repeats"
> +	gdb_test_no_output "set print characters $x" "set print characters $x repeats"
>  	for { set e 1 } { $e <= 16 } {incr e } {
>  	    set v [expr $e - 1]
>  	    set command "p &ctable2\[${v}*16\]"
> @@ -478,7 +478,7 @@ proc test_print_repeats_10 {} {
>  		set xstr "${xstr}\[.\]\[.\]\[.\]"
>  	    }
>  	    set string " = \[(\]unsigned char \[*\]\[)\] <ctable2(\\+$decimal)?> ${a}${xstr}"
> -	    gdb_test "$command" "$string" "$command with print elements set to $x"
> +	    gdb_test "$command" "$string" "$command with print characters set to $x"
>  	}
>      }
>  }
> @@ -503,26 +503,26 @@ proc test_print_strings {} {
>  
>      # Test that setting print elements unlimited doesn't completely suppress
>      # printing; this was a bug in older gdb's.
> -    gdb_test_no_output "set print elements 0"
> +    gdb_test_no_output "set print characters 0"
>      gdb_test "p teststring" \
> -	" = (.unsigned char .. )?\"teststring contents\"" "p teststring with elements set to 0"
> -    gdb_test_no_output "set print elements 1"
> +	" = (.unsigned char .. )?\"teststring contents\"" "p teststring with characters set to 0"
> +    gdb_test_no_output "set print characters 1"
>      gdb_test "p teststring" \
> -	" = (.unsigned char .. )?\"t\"\\.\\.\\." "p teststring with elements set to 1"
> -    gdb_test_no_output "set print elements 5"
> +	" = (.unsigned char .. )?\"t\"\\.\\.\\." "p teststring with characters set to 1"
> +    gdb_test_no_output "set print characters 5"
>      gdb_test "p teststring" \
> -	" = (.unsigned char .. )?\"tests\"\\.\\.\\." "p teststring with elements set to 5"
> -    gdb_test_no_output "set print elements 19"
> +	" = (.unsigned char .. )?\"tests\"\\.\\.\\." "p teststring with characters set to 5"
> +    gdb_test_no_output "set print characters 19"
>      gdb_test "p teststring" \
> -	" = (.unsigned char .. )?\"teststring contents\"" "p teststring with elements set to 19"
> -    gdb_test_no_output "set print elements 20"
> +	" = (.unsigned char .. )?\"teststring contents\"" "p teststring with characters set to 19"
> +    gdb_test_no_output "set print characters 20"
>      gdb_test "p teststring" \
> -	" = (.unsigned char .. )?\"teststring contents\"" "p teststring with elements set to 20"
> +	" = (.unsigned char .. )?\"teststring contents\"" "p teststring with characters set to 20"
>  
>      gdb_test "print teststring2" \
>  	" = \\(charptr\\) \"more contents\""
>  
> -    gdb_test_no_output "set print elements 8"
> +    gdb_test_no_output "set print characters 8"
>  
>      # Set the target-charset to ASCII, because the output varies from
>      # different charset.
> @@ -693,7 +693,7 @@ proc test_print_char_arrays {} {
>  proc test_print_string_constants {} {
>      global gdb_prompt
>  
> -    gdb_test_no_output "set print elements 50"
> +    gdb_test_no_output "set print characters 50"
>  
>      if [target_info exists gdb,cannot_call_functions] {
>  	unsupported "this target can not call functions"
> Index: src/gdb/testsuite/gdb.base/wchar.exp
> ===================================================================
> --- src.orig/gdb/testsuite/gdb.base/wchar.exp
> +++ src/gdb/testsuite/gdb.base/wchar.exp
> @@ -62,13 +62,13 @@ gdb_test_no_output "set print null on"
>  gdb_test "print repeat" "= L\"A\", '$cent' <repeats 21 times>, \"B\"" \
>      "print repeat (print null on)"
>  
> -gdb_test_no_output "set print elements 3"
> +gdb_test_no_output "set print characters 3"
>  
>  gdb_test "print repeat" "= L\"A$cent$cent\"\.\.\." \
> -    "print repeat (print elements 3)"
> +    "print repeat (print characters 3)"
>  
>  gdb_test "print repeat_p" "= $hex L\"A$cent$cent\"\.\.\." \
> -    "print repeat_p (print elements 3)"
> +    "print repeat_p (print characters 3)"
>  
>  # From PR cli/14977, but here because it requires wchar_t.
>  gdb_test "printf \"%ls\\n\", 0" "\\(null\\)"
> Index: src/gdb/testsuite/gdb.base/with.exp
> ===================================================================
> --- src.orig/gdb/testsuite/gdb.base/with.exp
> +++ src/gdb/testsuite/gdb.base/with.exp
> @@ -252,7 +252,7 @@ with_test_prefix "errors" {
>      gdb_test "with print elements 1 -- unknowncommand" \
>  	"Undefined command: \"unknowncommand\"\\.  Try \"help\"\\."
>      gdb_test "show print elements" \
> -	"Limit on string chars or array elements to print is 200\\."
> +	"Limit on array elements to print is 200\\."
>  }
>  
>  # Check completion.
> Index: src/gdb/testsuite/gdb.guile/scm-pretty-print.exp
> ===================================================================
> --- src.orig/gdb/testsuite/gdb.guile/scm-pretty-print.exp
> +++ src/gdb/testsuite/gdb.guile/scm-pretty-print.exp
> @@ -76,13 +76,15 @@ proc run_lang_tests {exefile lang} {
>  		" = \{.*<Vbase1> = pp class name: Vbase1.*<Vbase2> = \{.*<VirtualTest> = pp value variable is: 1,.*members of Vbase2:.*_vptr.Vbase2 = $hex.*<Vbase3> = \{.*members of Vbase3.*members of Derived:.*value = 2.*"
>  	    gdb_test "print ns " "\"embedded\\\\000null\\\\000string\"" \
>  		"print ns with 200 elements"
> -	    gdb_test_no_output "set print elements 3" ""
> +	    gdb_test_no_output "set print characters 3" ""
>  	    gdb_test "print ns" "emb\.\.\.." \
> -		"print ns with 3 elements"
> -	    gdb_test_no_output "set print elements 10" ""
> +		"print ns with 3 characters"
> +	    gdb_test "print -characters 4 -- ns" "embe\.\.\.." \
> +		"print ns with 4 characters"
> +	    gdb_test_no_output "set print characters 10" ""
>  	    gdb_test "print ns" "embedded\\\\000n\.\.\.." \
> -		"print ns with 10 elements"
> -	    gdb_test_no_output "set print elements 200" ""
> +		"print ns with 10 characters"
> +	    gdb_test_no_output "set print characters 200" ""
>  	}
>  
>  	if { ![is_address_zero_readable] } {
> Index: src/gdb/testsuite/gdb.pascal/str-chars.exp
> ===================================================================
> --- /dev/null
> +++ src/gdb/testsuite/gdb.pascal/str-chars.exp
> @@ -0,0 +1,48 @@
> +# Copyright 2021 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +load_lib "pascal.exp"
> +
> +standard_testfile .pas
> +
> +if {[gdb_compile_pascal "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug ]] != "" } {
> +  return -1
> +}
> +
> +clean_restart ${testfile}
> +gdb_breakpoint ${srcfile}:[gdb_get_line_number "set breakpoint 1 here"]
> +
> +# Verify that "start" lands inside the right procedure.
> +if { [gdb_start_cmd] < 0 } {
> +    untested start
> +    return -1
> +}
> +
> +gdb_continue_to_breakpoint "continue to breakpoint"
> +
> +gdb_test "print message" " = 'abcdefghijklmnopqrstuvwxyz'" \
> +    "print message with the default settings"
> +
> +gdb_test_no_output "set print elements 10"
> +gdb_test "print message" " = 'abcdefghijklmnopqrstuvwxyz'" \
> +    "print message with 'print elements' set to 10"
> +
> +gdb_test_no_output "set print characters 10"
> +gdb_test "print message" " = 'abcdefghij'\\.\\.\\." \
> +    "print message with 'print characters' set to 10"
> +
> +gdb_test_no_output "set print characters unlimited"
> +gdb_test "print message" " = 'abcdefghijklmnopqrstuvwxyz'" \
> +    "print message with 'print characters' set to unlimited"
> Index: src/gdb/testsuite/gdb.pascal/str-chars.pas
> ===================================================================
> --- /dev/null
> +++ src/gdb/testsuite/gdb.pascal/str-chars.pas
> @@ -0,0 +1,28 @@
> +{
> + Copyright 2021 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +}
> +
> +program str_char;
> +
> +var
> +   message : string;
> +
> +begin
> +
> +   message := 'abcdefghijklmnopqrstuvwxyz';
> +   writeln (message)	{ set breakpoint 1 here }
> +
> +end.
> Index: src/gdb/testsuite/gdb.python/py-format-string.exp
> ===================================================================
> --- src.orig/gdb/testsuite/gdb.python/py-format-string.exp
> +++ src/gdb/testsuite/gdb.python/py-format-string.exp
> @@ -632,7 +632,7 @@ proc_with_prefix test_max_elements {} {
>  
>    # 200 is the default maximum number of elements, so setting it should
>    # not change the output.
> -  set opts "max_elements=200"
> +  set opts "max_elements=200, max_characters=200"
>    with_test_prefix $opts {
>      check_format_string "a_point_t" $opts
>      check_format_string "a_point_t_pointer" $opts
> @@ -653,7 +653,7 @@ proc_with_prefix test_max_elements {} {
>      }
>    }
>  
> -  set opts "max_elements=3"
> +  set opts "max_elements=3, max_characters=3"
>    with_test_prefix $opts {
>      check_format_string "a_point_t" $opts
>      check_format_string "a_point_t_pointer" $opts
> @@ -683,7 +683,7 @@ proc_with_prefix test_max_elements {} {
>  
>    # Both 1,000 (we don't have that many elements) and 0 (unlimited) should
>    # mean no truncation.
> -  foreach opts { "max_elements=1000" "max_elements=0" } {
> +  foreach opts { "max_elements=1000, max_characters=1000" "max_elements=0, max_characters=0" } {
>      with_test_prefix $opts {
>        check_format_string "a_point_t" $opts
>        check_format_string "a_point_t_pointer" $opts
> @@ -706,13 +706,16 @@ proc_with_prefix test_max_elements {} {
>      }
>    }
>  
> -  with_temp_option "set print elements 4" "set print elements 200" {
> +  with_temp_option "set print characters 4" "set print characters 200" {
>      check_format_string "a_string" "" \
>        "${default_pointer_regexp} \"hell\"..."
>      check_format_string "a_binary_string" "" \
>        "${default_pointer_regexp} \"hell\"..."
>      check_format_string "a_binary_string_array" "" \
>        "\"hell\"..."
> +  }
> +
> +  with_temp_option "set print elements 4" "set print elements 200" {
>      check_format_string "an_array_with_repetition" "" \
>        "\\{1, 3 <repeats 12 times>...\\}"
>    }
> Index: src/gdb/testsuite/gdb.python/py-prettyprint.exp
> ===================================================================
> --- src.orig/gdb/testsuite/gdb.python/py-prettyprint.exp
> +++ src/gdb/testsuite/gdb.python/py-prettyprint.exp
> @@ -80,13 +80,15 @@ proc run_lang_tests {exefile lang} {
>  	    " = \{.*<Vbase1> = pp class name: Vbase1.*<Vbase2> = \{.*<VirtualTest> = pp value variable is: 1,.*members of Vbase2:.*_vptr.Vbase2 = $hex.*<Vbase3> = \{.*members of Vbase3.*members of Derived:.*value = 2.*"
>  	gdb_test "print ns " "\"embedded\\\\000null\\\\000string\"" \
>  	    "print ns with default element limit"
> -	gdb_test_no_output "set print elements 3"
> +	gdb_test_no_output "set print characters 3"
>  	gdb_test "print ns" "emb\.\.\.." \
>  	    "print ns with element limit of 3"
> -	gdb_test_no_output "set print elements 10"
> +	gdb_test "print -characters 4 -- ns" "embe\.\.\.." \
> +	    "print ns with element limit of 4"
> +	gdb_test_no_output "set print characters 10"
>  	gdb_test "print ns" "embedded\\\\000n\.\.\.." \
>  	    "print ns with element limit of 10"
> -	gdb_test_no_output "set print elements 200"
> +	gdb_test_no_output "set print characters 200"
>      }
>  
>      if { ![is_address_zero_readable] } {
> Index: src/gdb/testsuite/lib/gdb.exp
> ===================================================================
> --- src.orig/gdb/testsuite/lib/gdb.exp
> +++ src/gdb/testsuite/lib/gdb.exp
> @@ -6443,7 +6443,7 @@ gdb_caching_proc gdb_has_argv0 {
>  	set old_elements "200"
>  	set test "show print elements"
>  	gdb_test_multiple $test $test {
> -	    -re "Limit on string chars or array elements to print is (\[^\r\n\]+)\\.\r\n$gdb_prompt $" {
> +	    -re "Limit on array elements to print is (\[^\r\n\]+)\\.\r\n$gdb_prompt $" {
>  		set old_elements $expect_out(1,string)
>  	    }
>  	}
> Index: src/gdb/tracepoint.c
> ===================================================================
> --- src.orig/gdb/tracepoint.c
> +++ src/gdb/tracepoint.c
> @@ -545,9 +545,9 @@ decode_agent_options (const char *exp, i
>        if (target_supports_string_tracing ())
>  	{
>  	  /* Allow an optional decimal number giving an explicit maximum
> -	     string length, defaulting it to the "print elements" value;
> +	     string length, defaulting it to the "print characters" value;
>  	     so "collect/s80 mystr" gets at most 80 bytes of string.  */
> -	  *trace_string = opts.print_max;
> +	  *trace_string = opts.print_max_chars;
>  	  exp++;
>  	  if (*exp >= '0' && *exp <= '9')
>  	    *trace_string = atoi (exp);
> Index: src/gdb/valprint.c
> ===================================================================
> --- src.orig/gdb/valprint.c
> +++ src/gdb/valprint.c
> @@ -97,8 +97,11 @@ static void val_print_type_code_flags (s
>  				       int embedded_offset,
>  				       struct ui_file *stream);
>  
> -#define PRINT_MAX_DEFAULT 200	/* Start print_max off at this value.  */
> -#define PRINT_MAX_DEPTH_DEFAULT 20	/* Start print_max_depth off at this value. */
> +/* Start print_max and print_max_chars at this value.  */
> +#define PRINT_MAX_DEFAULT 200
> +
> +/* Start print_max_depth at this value. */
> +#define PRINT_MAX_DEPTH_DEFAULT 20
>  
>  struct value_print_options user_print_options =
>  {
> @@ -110,6 +113,7 @@ struct value_print_options user_print_op
>    1,				/* addressprint */
>    0,				/* objectprint */
>    PRINT_MAX_DEFAULT,		/* print_max */
> +  PRINT_MAX_DEFAULT,		/* print_max_chars */
>    10,				/* repeat_count_threshold */
>    0,				/* output_format */
>    0,				/* format */
> @@ -152,13 +156,25 @@ get_formatted_print_options (struct valu
>    opts->format = format;
>  }
>  
> +/* Implement 'show print elements'.  */
> +
>  static void
>  show_print_max (struct ui_file *file, int from_tty,
>  		struct cmd_list_element *c, const char *value)
>  {
>    fprintf_filtered (file,
> -		    _("Limit on string chars or array "
> -		      "elements to print is %s.\n"),
> +		    _("Limit on array elements to print is %s.\n"),
> +		    value);
> +}
> +
> +/* Implement 'show print characters'.  */
> +
> +static void
> +show_print_max_chars (struct ui_file *file, int from_tty,
> +		      struct cmd_list_element *c, const char *value)
> +{
> +  fprintf_filtered (file,
> +		    _("Limit on string characters to print is %s.\n"),
>  		    value);
>  }
>  
> @@ -2601,9 +2617,9 @@ print_converted_chars_to_obstack (struct
>  /* Print the character string STRING, printing at most LENGTH
>     characters.  LENGTH is -1 if the string is nul terminated.  TYPE is
>     the type of each character.  OPTIONS holds the printing options;
> -   printing stops early if the number hits print_max; repeat counts
> -   are printed as appropriate.  Print ellipses at the end if we had to
> -   stop before printing LENGTH characters, or if FORCE_ELLIPSES.
> +   printing stops early if the number hits print_max_chars; repeat
> +   counts are printed as appropriate.  Print ellipses at the end if we
> +   had to stop before printing LENGTH characters, or if FORCE_ELLIPSES.
>     QUOTE_CHAR is the character to print at each end of the string.  If
>     C_STYLE_TERMINATOR is true, and the last character is 0, then it is
>     omitted.  */
> @@ -2657,7 +2673,7 @@ generic_printstr (struct ui_file *stream
>    /* Convert characters until the string is over or the maximum
>       number of printed characters has been reached.  */
>    i = 0;
> -  while (i < options->print_max)
> +  while (i < options->print_max_chars)
>      {
>        int r;
>  
> @@ -2709,7 +2725,7 @@ generic_printstr (struct ui_file *stream
>  /* Print a string from the inferior, starting at ADDR and printing up to LEN
>     characters, of WIDTH bytes a piece, to STREAM.  If LEN is -1, printing
>     stops at the first null byte, otherwise printing proceeds (including null
> -   bytes) until either print_max or LEN characters have been printed,
> +   bytes) until either print_max_chars or LEN characters have been printed,
>     whichever is smaller.  ENCODING is the name of the string's
>     encoding.  It can be NULL, in which case the target encoding is
>     assumed.  */
> @@ -2731,15 +2747,16 @@ val_print_string (struct type *elttype,
>    int width = TYPE_LENGTH (elttype);
>  
>    /* First we need to figure out the limit on the number of characters we are
> -     going to attempt to fetch and print.  This is actually pretty simple.  If
> -     LEN >= zero, then the limit is the minimum of LEN and print_max.  If
> -     LEN is -1, then the limit is print_max.  This is true regardless of
> -     whether print_max is zero, UINT_MAX (unlimited), or something in between,
> -     because finding the null byte (or available memory) is what actually
> -     limits the fetch.  */
> +     going to attempt to fetch and print.  This is actually pretty simple.
> +     If LEN >= zero, then the limit is the minimum of LEN and print_max_chars.
> +     If LEN is -1, then the limit is print_max_chars.  This is true regardless
> +     of whether print_max_chars is zero, UINT_MAX (unlimited), or something in
> +     between, because finding the null byte (or available memory) is what
> +     actually limits the fetch.  */
>  
> -  fetchlimit = (len == -1 ? options->print_max : std::min ((unsigned) len,
> -							   options->print_max));
> +  fetchlimit = (len == -1
> +		? options->print_max_chars
> +		: std::min ((unsigned) len, options->print_max_chars));
>  
>    err = read_string (addr, len, width, fetchlimit, byte_order,
>  		     &buffer, &bytes_read);
> @@ -3020,8 +3037,17 @@ static const gdb::option::option_def val
>      "elements",
>      [] (value_print_options *opt) { return &opt->print_max; },
>      show_print_max, /* show_cmd_cb */
> -    N_("Set limit on string chars or array elements to print."),
> -    N_("Show limit on string chars or array elements to print."),
> +    N_("Set limit on array elements to print."),
> +    N_("Show limit on array elements to print."),
> +    N_("\"unlimited\" causes there to be no limit."),
> +  },
> +
> +  uinteger_option_def {
> +    "characters",
> +    [] (value_print_options *opt) { return &opt->print_max_chars; },
> +    show_print_max_chars, /* show_cmd_cb */
> +    N_("Set limit on string chars to print."),
> +    N_("Show limit on string chars to print."),
>      N_("\"unlimited\" causes there to be no limit."),
>    },
>  
> Index: src/gdb/valprint.h
> ===================================================================
> --- src.orig/gdb/valprint.h
> +++ src/gdb/valprint.h
> @@ -48,12 +48,14 @@ struct value_print_options
>       in its vtables.  */
>    bool objectprint;
>  
> -  /* Maximum number of chars to print for a string pointer value or vector
> -     contents, or UINT_MAX for no limit.  Note that "set print elements 0"
> -     stores UINT_MAX in print_max, which displays in a show command as
> -     "unlimited".  */
> +  /* Maximum number of elements to print for vector contents, or UINT_MAX
> +     for no limit.  Note that "set print elements 0" stores UINT_MAX in
> +     print_max, which displays in a show command as "unlimited".  */
>    unsigned int print_max;
>  
> +  /* Maximum number of string chars to print for a string pointer value.  */
> +  unsigned int print_max_chars;
> +
>    /* Print repeat counts if there are more than this many repetitions
>       of an element in an array.  */
>    unsigned int repeat_count_threshold;
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gdb: split array and string limiting options
  2021-12-15 18:01 ` Andrew Burgess
@ 2022-01-04 23:15   ` Maciej W. Rozycki
  0 siblings, 0 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2022-01-04 23:15 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches, Simon Marchi

On Wed, 15 Dec 2021, Andrew Burgess wrote:

> > Index: src/gdb/NEWS
> > ===================================================================
> > --- src.orig/gdb/NEWS
> > +++ src/gdb/NEWS
> > @@ -53,6 +53,24 @@ maint packet
> >    as escaped hex, e.g. \x?? where '??' is replaces with the value of
> >    the non-printable character.
> >  
> > +set print characters LIMIT
> > +show print characters
> > +print -characters LIMIT
> > +  This new setting is like 'set print elements', but controls how many
> > +  characters of a string are printed.  This functionality used to be
> > +  covered by 'set print elements', but is now a separate setting.
> > +  LIMIT can be set to 'unlimited' to print all characters of a string.
> > +  The default value for LIMIT is 200.
> > +
> > +* Changed commands
> > +
> > +set print elements LIMIT
> > +show print elements
> > +print -elements LIMIT
> > +  This setting no longer affects how many characters of a string are
> > +  printed, instead the new 'set print characters' setting should be
> > +  used.
> > +
> >  * Python API
> 
> I suspect something went wrong with the rebase here.  The 'set print
> characters' block should be under 'New commands', and the 'set print
> elements' block should, indeed, be under 'Changed commands'.  But, I
> think the 'Changed commands' you're adding here is now the second such
> title for the current release of GDB.

 Indeed, good catch!  Thank you, I have posted v3 with this piece fixed 
only.

  Maciej

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gdb: split array and string limiting options
  2022-01-26 16:52       ` Simon Sobisch
@ 2022-01-26 18:18         ` Maciej W. Rozycki
  0 siblings, 0 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2022-01-26 18:18 UTC (permalink / raw)
  To: Simon Sobisch; +Cc: Andrew Burgess, gdb-patches

On Wed, 26 Jan 2022, Simon Sobisch wrote:

> > > > > To do so the new parameter cannot be directly used as it is in the
> > > > > patch
> > > > > but must be fowarded to a new function or duplicated.
> > > > > 
> > > > > As an example this would mean instead of :
> > > > > 
> > > > > --- src.orig/gdb/c-valprint.c
> > > > > +++ src/gdb/c-valprint.c
> > > > > @@ -271,7 +271,7 @@ c_value_print_array (struct value *val,
> > > > > 
> > > > >       for (temp_len = 0;
> > > > >    	   (temp_len < len
> > > > > -	    && temp_len < options->print_max
> > > > > +	    && temp_len < options->print_max_chars
> > > > >    	    && extract_unsigned_integer (valaddr + temp_len * eltlen,
> > > > >    					 eltlen, byte	_order) != 0);
> > > > >    		   ++temp_len)
> > > > > 
> > > > > have the following
> > > > > 
> > > > > 
> > > > > +   const int print_max_chars = (options->print_max_chars != -1) ?
> > > > > options->print_max_chars : options->print_max;
> > > > >       for (temp_len = 0;
> > > > >    	   (temp_len < len
> > > > > -	    && temp_len < options->print_max
> > > > > +	    && temp_len < print_max_chars
> > > > >    	    && extract_unsigned_integer (valaddr + temp_len * eltlen,
> > > > >    					 eltlen, byte_order) != 0);
> > > > >    		   ++temp_len)
> > > > 
> 
> Proposal 1 - Should work, shouldn't it?
> 
> >   If you want to keep backwards compatibility, then no particular value for
> > `set print elements' can be used to drive `set print characters' I'm
> > afraid.  Someone may have used it somewhere.
> 
> What do you mean with that? If the default is whatever print elements is it
> should be compatible, shouldn't it?

 Right, I got this mixed up as it's the new `set print characters' setting 
that will drive the default rather than the existing `set print elements' 
one, the semantics of which will remain unchanged for non-character 
entities.  So e.g.:

(gdb) set print characters elements

(which will also be the default) will make `set print elements' drive the 
number of characters printed (and the `elements' syntactic argument can 
either set the limit to a reserved value such as `-1' as you suggested or 
drive a separate internal boolean flag).  While if we use any other value, 
then the character limit will get separated from the `set print elements' 
setting.

 So it seems good to me and probably the least interface complication for 
users and the least maintenance burden to carry along.  Thank you for your 
persistence!

 Any other input, anyone?

  Maciej

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gdb: split array and string limiting options
  2022-01-26 15:48     ` Maciej W. Rozycki
@ 2022-01-26 16:52       ` Simon Sobisch
  2022-01-26 18:18         ` Maciej W. Rozycki
  0 siblings, 1 reply; 11+ messages in thread
From: Simon Sobisch @ 2022-01-26 16:52 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Andrew Burgess, gdb-patches


Am 26.01.2022 um 16:48 schrieb Maciej W. Rozycki:
> Hi Simon,
> 
>   Your message wasn't directly cc-ed to me and I missed it previously in
> the flood of mailing-list traffic.  Sorry about this.

Thanks for still coming back :-)

>   Please note that I have posted v3 before your message already.

I see, so some of my questions are possibly also not up-tp-date-

>>>> In my opinion there should be only 1 new test for the new setting and
>>>> the other should be left unchanged while still working, because then
>>>> existing usages will still work, too.
> 
>   Agreed.

That's something we can work with - let's check the implementations and 
options below for that.

> 
>>>> To do so the new parameter cannot be directly used as it is in the patch
>>>> but must be fowarded to a new function or duplicated.
>>>>
>>>> As an example this would mean instead of :
>>>>
>>>> --- src.orig/gdb/c-valprint.c
>>>> +++ src/gdb/c-valprint.c
>>>> @@ -271,7 +271,7 @@ c_value_print_array (struct value *val,
>>>>
>>>>       for (temp_len = 0;
>>>>    	   (temp_len < len
>>>> -	    && temp_len < options->print_max
>>>> +	    && temp_len < options->print_max_chars
>>>>    	    && extract_unsigned_integer (valaddr + temp_len * eltlen,
>>>>    					 eltlen, byte	_order) != 0);
>>>>    		   ++temp_len)
>>>>
>>>> have the following
>>>>
>>>>
>>>> +   const int print_max_chars = (options->print_max_chars != -1) ?
>>>> options->print_max_chars : options->print_max;
>>>>       for (temp_len = 0;
>>>>    	   (temp_len < len
>>>> -	    && temp_len < options->print_max
>>>> +	    && temp_len < print_max_chars
>>>>    	    && extract_unsigned_integer (valaddr + temp_len * eltlen,
>>>>    					 eltlen, byte_order) != 0);
>>>>    		   ++temp_len)
>>>

Proposal 1 - Should work, shouldn't it?

>   If you want to keep backwards compatibility, then no particular value for
> `set print elements' can be used to drive `set print characters' I'm
> afraid.  Someone may have used it somewhere.

What do you mean with that? If the default is whatever print elements is 
it should be compatible, shouldn't it?

Please elaborate. Also: is that option "still on the table"?

>   What I can offer as possible alternative solutions are:
> 
> * As you proposed, but instead driven by an entirely separate knob such as
>    `set print characters-enable <on|off>'; if "off", which would also be
>    the default, then keeping the current semantics of `set print elements'.

Proposal 2 - That's an interesting idea. We could also say that in a 
later GDB version the default will be changed to on.
The best thing here: it is easily possible to have a different default 
for print characters (possibly much higher than print elements) with 
still keeping backward compatibility.

> * Leave `set print elements' alone and instead introduce a replacement
>    say `set print array-elements' command for array elements in addition to
>    `set print characters' already have proposed.  The latter two commands
>    would adjust the respective settings each while the former command would
>    set them both, for compatibility.  It's not clear to me what `show print
>    elements' would have to report however, maybe the `print array-elements'
>    setting.

Proposal 3 - also an interesting idea, a clean split.
I tend to this one.
> 
> * Have `set print elements' adjust both limits till `set print characters'
>    or `show print characters' have been used, at which point further
>    requests would only adjust the array element limit.  This wouldn't pose
>    a problem with `show print elements', but the semantics would be
>    complicated.

Proposal 4 - has "complicated in"  - let's drop it.

>   Thoughts?
> 
>    Maciej

Thanks again for working on this,
Simon

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gdb: split array and string limiting options
  2022-01-06 13:32   ` Simon Sobisch
  2022-01-06 14:18     ` Andrew Burgess
@ 2022-01-26 15:48     ` Maciej W. Rozycki
  2022-01-26 16:52       ` Simon Sobisch
  1 sibling, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2022-01-26 15:48 UTC (permalink / raw)
  To: Simon Sobisch; +Cc: Andrew Burgess, gdb-patches

Hi Simon,

 Your message wasn't directly cc-ed to me and I missed it previously in 
the flood of mailing-list traffic.  Sorry about this.

 Please note that I have posted v3 before your message already.

> > > I really like the general idea and would like to see this added, but I
> > > think the patch goes too far and will break both old and new usage.
> > 
> > I don't understand what "new usage" means here.
> 
> Thanks for checking, that was unclear. With "new usage" I've meant that if
> someone temporarily sets the new variable in an extension there is no way to
> get back. The "-1 magic value" I've suggested would also allow a temporary
> setting outside of "with print characters ".
> 
> That actually reminds me of a documentation question I had: Does the
> implementation actually return the amount of characters (especially when the
> char array which is printed from has multiple multi-byte characters with
> combining symbols)?

 It does, see the bottom of my message at: 
<https://sourceware.org/pipermail/gdb-patches/2021-December/184466.html>.

> > > In my opinion there should be only 1 new test for the new setting and
> > > the other should be left unchanged while still working, because then
> > > existing usages will still work, too.

 Agreed.

> > > To do so the new parameter cannot be directly used as it is in the patch
> > > but must be fowarded to a new function or duplicated.
> > > 
> > > As an example this would mean instead of :
> > > 
> > > --- src.orig/gdb/c-valprint.c
> > > +++ src/gdb/c-valprint.c
> > > @@ -271,7 +271,7 @@ c_value_print_array (struct value *val,
> > > 
> > >      for (temp_len = 0;
> > >   	   (temp_len < len
> > > -	    && temp_len < options->print_max
> > > +	    && temp_len < options->print_max_chars
> > >   	    && extract_unsigned_integer (valaddr + temp_len * eltlen,
> > >   					 eltlen, byte	_order) != 0);
> > >   		   ++temp_len)
> > > 
> > > have the following
> > > 
> > > 
> > > +   const int print_max_chars = (options->print_max_chars != -1) ?
> > > options->print_max_chars : options->print_max;
> > >      for (temp_len = 0;
> > >   	   (temp_len < len
> > > -	    && temp_len < options->print_max
> > > +	    && temp_len < print_max_chars
> > >   	    && extract_unsigned_integer (valaddr + temp_len * eltlen,
> > >   					 eltlen, byte_order) != 0);
> > >   		   ++temp_len)
> > 
> > What follows is just my thoughts on what you propose.  This work was
> > being done for a client of Embecosm's, and Maciej is now driving this
> > patch, so what he does is up to him.  That said...
> > 
> > I did consider this when I originally wrote the patch.  But decided
> > against it in the end.
> > 
> > I agree that maintaining backwards compatibility is important.
> 
> :-)

 If you want to keep backwards compatibility, then no particular value for 
`set print elements' can be used to drive `set print characters' I'm 
afraid.  Someone may have used it somewhere.

 What I can offer as possible alternative solutions are:

* As you proposed, but instead driven by an entirely separate knob such as 
  `set print characters-enable <on|off>'; if "off", which would also be 
  the default, then keeping the current semantics of `set print elements'.

* Leave `set print elements' alone and instead introduce a replacement
  say `set print array-elements' command for array elements in addition to 
  `set print characters' already have proposed.  The latter two commands 
  would adjust the respective settings each while the former command would 
  set them both, for compatibility.  It's not clear to me what `show print 
  elements' would have to report however, maybe the `print array-elements' 
  setting.

* Have `set print elements' adjust both limits till `set print characters' 
  or `show print characters' have been used, at which point further 
  requests would only adjust the array element limit.  This wouldn't pose 
  a problem with `show print elements', but the semantics would be 
  complicated.

 Thoughts?

  Maciej

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gdb: split array and string limiting options
  2022-01-06 13:32   ` Simon Sobisch
@ 2022-01-06 14:18     ` Andrew Burgess
  2022-01-26 15:48     ` Maciej W. Rozycki
  1 sibling, 0 replies; 11+ messages in thread
From: Andrew Burgess @ 2022-01-06 14:18 UTC (permalink / raw)
  To: Simon Sobisch; +Cc: gdb-patches

* Simon Sobisch via Gdb-patches <gdb-patches@sourceware.org> [2022-01-06 14:32:02 +0100]:

> 
> Thanks for the initial work on the patch, for sharing your thoughts and
> request for clarification. I hope Maciej will find the time to work on this
> and that we may see this "earlier" than GDB 12.

For the record, I will pick this up if Maciej doesn't, but given he's
already "claimed" this work[1] I'm not going to step on his toes.

Thanks,
Andrew

[1] https://sourceware.org/pipermail/gdb-patches/2021-November/183015.html


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gdb: split array and string limiting options
  2022-01-06  9:33 ` Andrew Burgess
@ 2022-01-06 13:32   ` Simon Sobisch
  2022-01-06 14:18     ` Andrew Burgess
  2022-01-26 15:48     ` Maciej W. Rozycki
  0 siblings, 2 replies; 11+ messages in thread
From: Simon Sobisch @ 2022-01-06 13:32 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches



Am 06.01.2022 um 10:33 schrieb Andrew Burgess:
> * Simon Sobisch via Gdb-patches <gdb-patches@sourceware.org> [2022-01-06 08:58:38 +0100]:
> 
>>> From: Andrew Burgess <andrew.burgess@embecosm.com>
>>>
>>> This commit splits the set/show print elements options into two.  We
>>> retain set/show print elements for controlling how many elements of an
>>> array we print, but a new set/show print characters is added which is
>>> used for controlling how many characters of a string are printed.
>>> [...]
>> Reference to the full patch:
>> https://sourceware.org/pipermail/gdb-patches/2021-December/184467.html
>>
>>
>> I really like the general idea and would like to see this added, but I
>> think the patch goes too far and will break both old and new usage.
> 
> I don't understand what "new usage" means here.

Thanks for checking, that was unclear. With "new usage" I've meant that 
if someone temporarily sets the new variable in an extension there is no 
way to get back. The "-1 magic value" I've suggested would also allow a 
temporary setting outside of "with print characters ".

That actually reminds me of a documentation question I had: Does the 
implementation actually return the amount of characters (especially when 
the char array which is printed from has multiple multi-byte characters 
with combining symbols)?

>> In my opinion there should be only 1 new test for the new setting and
>> the other should be left unchanged while still working, because then
>> existing usages will still work, too.
>>
>> To do so the new parameter cannot be directly used as it is in the patch
>> but must be fowarded to a new function or duplicated.
>>
>> As an example this would mean instead of :
>>
>> --- src.orig/gdb/c-valprint.c
>> +++ src/gdb/c-valprint.c
>> @@ -271,7 +271,7 @@ c_value_print_array (struct value *val,
>>
>>      for (temp_len = 0;
>>   	   (temp_len < len
>> -	    && temp_len < options->print_max
>> +	    && temp_len < options->print_max_chars
>>   	    && extract_unsigned_integer (valaddr + temp_len * eltlen,
>>   					 eltlen, byte	_order) != 0);
>>   		   ++temp_len)
>>
>> have the following
>>
>>
>> +   const int print_max_chars = (options->print_max_chars != -1) ?
>> options->print_max_chars : options->print_max;
>>      for (temp_len = 0;
>>   	   (temp_len < len
>> -	    && temp_len < options->print_max
>> +	    && temp_len < print_max_chars
>>   	    && extract_unsigned_integer (valaddr + temp_len * eltlen,
>>   					 eltlen, byte_order) != 0);
>>   		   ++temp_len)
> 
> What follows is just my thoughts on what you propose.  This work was
> being done for a client of Embecosm's, and Maciej is now driving this
> patch, so what he does is up to him.  That said...
> 
> I did consider this when I originally wrote the patch.  But decided
> against it in the end.
> 
> I agree that maintaining backwards compatibility is important.

:-)

> But I think we should be careful about going over the top to avoid breaking
> changes.

I think that depends on the actual implementation effort and additional 
technical dept introduced.

> IMHO GDB script is not equivalent to something like C, C++, Python,
> etc, where I would certainly expect complete backwards compatibility.

Commonly GDB _adds_ new options. This allows to improve GDB scripts to 
get the new features, but it would already be bad if existing scripts 
don't work any more.
The thing I'm much more interested in here isn't a simple GDB script but 
GDB extensions (Python, likely also Guile but I haven't worked with 
that) and MI-Clients. Those extensions can get quite large and breaking 
those is therefore much worse than breaking a simple GDB script.

It is not unlikely that you'll find some that temporarily set elements 
to a big value or even unlimited to present the user the full string, 
with the patch applied that's immediately broken as changing this value 
has no effect at all for the strings.

> Instead I think we should aim to strike a balance between maintaining
> backward compatibility, and keeping a simple, easy to understand
> interface.

Agreed.

> For me, changing the behaviour of this option didn't seem that crazy,
> it's pretty easy to restore the old behaviour by also setting the new
> option.

... for which you'd need (as a "simple user") to wait for your MI-client 
or Python extension's author to change their software and hope you have 
access to the previous version of GDB to still be able to do your debugging.

>  Maybe the help text could do a better job of cross
> referencing the other option, to make discovery easier.

Possibly. With my suggested change the news entry would say that the new 
setting is undefined (or off = -1) and so the old setting is applied.

> Anyway, just me thoughts.  I'm certainly not going to block the patch
> if it is rewritten inline with your suggestion.

I hope it will be - and doing so will also heavily reduce the amount of 
changes in the patch - as all old testsuite entries can stay as they are.

>> This effectively means that until you explicit set the new "print
>> characters" the existing "print elements" setting will be used, and if
>> you "set print characters off" (or similar, that would internally
>> resolve to -1) you get the length depending on "print elements" again.
>>
>> The parameter type in python for "print elements" is (guessed)
>> gdb.PARAM_UINTEGER, the type for "print characters" would be
>> gdb.PARAM_INTEGER with -1 meaning "take the other parameter".
>>
>>
>> If those changes are done there is no breakage whatever which could also
>> speedup the inclusion s it hopefully lands in GDB 11.2.
> 
> Maybe I'm not understanding how releases work, but I thought 11.2 was
> a bug fix release over 11.1, and a future 12 branch would be the new
> release in which new features would land.  Is that not how things
> work?

Depends on the project. In GDB Announcements of minor versions the 
introduction commonly is "brings the following fixes and enhancements". 
In the past there were also bigger new features (for example related to 
python) and as long as enhancements don't break backward compatibility 
at all...
But I actually don't know if there is a policy about what goes in (now).

> Thanks,
> Andrew
> 

Thanks for the initial work on the patch, for sharing your thoughts and 
request for clarification. I hope Maciej will find the time to work on 
this and that we may see this "earlier" than GDB 12.

Simon

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gdb: split array and string limiting options
  2022-01-06  7:58 Simon Sobisch
@ 2022-01-06  9:33 ` Andrew Burgess
  2022-01-06 13:32   ` Simon Sobisch
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Burgess @ 2022-01-06  9:33 UTC (permalink / raw)
  To: Simon Sobisch; +Cc: gdb-patches

* Simon Sobisch via Gdb-patches <gdb-patches@sourceware.org> [2022-01-06 08:58:38 +0100]:

> > From: Andrew Burgess <andrew.burgess@embecosm.com>
> >
> > This commit splits the set/show print elements options into two.  We
> > retain set/show print elements for controlling how many elements of an
> > array we print, but a new set/show print characters is added which is
> > used for controlling how many characters of a string are printed.
> > [...]
> Reference to the full patch:
> https://sourceware.org/pipermail/gdb-patches/2021-December/184467.html
> 
> 
> I really like the general idea and would like to see this added, but I
> think the patch goes too far and will break both old and new usage.

I don't understand what "new usage" means here.

> 
> In my opinion there should be only 1 new test for the new setting and
> the other should be left unchanged while still working, because then
> existing usages will still work, too.
> 
> To do so the new parameter cannot be directly used as it is in the patch
> but must be fowarded to a new function or duplicated.
> 
> As an example this would mean instead of :
> 
> --- src.orig/gdb/c-valprint.c
> +++ src/gdb/c-valprint.c
> @@ -271,7 +271,7 @@ c_value_print_array (struct value *val,
> 
>     for (temp_len = 0;
>  	   (temp_len < len
> -	    && temp_len < options->print_max
> +	    && temp_len < options->print_max_chars
>  	    && extract_unsigned_integer (valaddr + temp_len * eltlen,
>  					 eltlen, byte_order) != 0);
>  		   ++temp_len)
> 
> have the following
> 
> 
> +   const int print_max_chars = (options->print_max_chars != -1) ?
> options->print_max_chars : options->print_max;
>     for (temp_len = 0;
>  	   (temp_len < len
> -	    && temp_len < options->print_max
> +	    && temp_len < print_max_chars
>  	    && extract_unsigned_integer (valaddr + temp_len * eltlen,
>  					 eltlen, byte_order) != 0);
>  		   ++temp_len)

What follows is just my thoughts on what you propose.  This work was
being done for a client of Embecosm's, and Maciej is now driving this
patch, so what he does is up to him.  That said...

I did consider this when I originally wrote the patch.  But decided
against it in the end.

I agree that maintaining backwards compatibility is important.  But I
think we should be careful about going over the top to avoid breaking
changes.

IMHO GDB script is not equivalent to something like C, C++, Python,
etc, where I would certainly expect complete backwards compatibility.

Instead I think we should aim to strike a balance between maintaining
backward compatibility, and keeping a simple, easy to understand
interface.

For me, changing the behaviour of this option didn't seem that crazy,
it's pretty easy to restore the old behaviour by also setting the new
option.  Maybe the help text could do a better job of cross
referencing the other option, to make discovery easier.

Anyway, just me thoughts.  I'm certainly not going to block the patch
if it is rewritten inline with your suggestion.

> 
> 
> This effectively means that until you explicit set the new "print
> characters" the existing "print elements" setting will be used, and if
> you "set print characters off" (or similar, that would internally
> resolve to -1) you get the length depending on "print elements" again.
> 
> The parameter type in python for "print elements" is (guessed)
> gdb.PARAM_UINTEGER, the type for "print characters" would be
> gdb.PARAM_INTEGER with -1 meaning "take the other parameter".
> 
> 
> If those changes are done there is no breakage whatever which could also
> speedup the inclusion s it hopefully lands in GDB 11.2.

Maybe I'm not understanding how releases work, but I thought 11.2 was
a bug fix release over 11.1, and a future 12 branch would be the new
release in which new features would land.  Is that not how things
work?

Thanks,
Andrew


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] gdb: split array and string limiting options
@ 2022-01-06  7:58 Simon Sobisch
  2022-01-06  9:33 ` Andrew Burgess
  0 siblings, 1 reply; 11+ messages in thread
From: Simon Sobisch @ 2022-01-06  7:58 UTC (permalink / raw)
  To: gdb-patches

 > From: Andrew Burgess <andrew.burgess@embecosm.com>
 >
 > This commit splits the set/show print elements options into two.  We
 > retain set/show print elements for controlling how many elements of an
 > array we print, but a new set/show print characters is added which is
 > used for controlling how many characters of a string are printed.
 > [...]
Reference to the full patch:
https://sourceware.org/pipermail/gdb-patches/2021-December/184467.html


I really like the general idea and would like to see this added, but I
think the patch goes too far and will break both old and new usage.

In my opinion there should be only 1 new test for the new setting and
the other should be left unchanged while still working, because then
existing usages will still work, too.

To do so the new parameter cannot be directly used as it is in the patch
but must be fowarded to a new function or duplicated.

As an example this would mean instead of :

--- src.orig/gdb/c-valprint.c
+++ src/gdb/c-valprint.c
@@ -271,7 +271,7 @@ c_value_print_array (struct value *val,

     for (temp_len = 0;
  	   (temp_len < len
-	    && temp_len < options->print_max
+	    && temp_len < options->print_max_chars
  	    && extract_unsigned_integer (valaddr + temp_len * eltlen,
  					 eltlen, byte_order) != 0);
  		   ++temp_len)

have the following


+   const int print_max_chars = (options->print_max_chars != -1) ?
options->print_max_chars : options->print_max;
     for (temp_len = 0;
  	   (temp_len < len
-	    && temp_len < options->print_max
+	    && temp_len < print_max_chars
  	    && extract_unsigned_integer (valaddr + temp_len * eltlen,
  					 eltlen, byte_order) != 0);
  		   ++temp_len)


This effectively means that until you explicit set the new "print
characters" the existing "print elements" setting will be used, and if
you "set print characters off" (or similar, that would internally
resolve to -1) you get the length depending on "print elements" again.

The parameter type in python for "print elements" is (guessed)
gdb.PARAM_UINTEGER, the type for "print characters" would be
gdb.PARAM_INTEGER with -1 meaning "take the other parameter".


If those changes are done there is no breakage whatever which could also
speedup the inclusion s it hopefully lands in GDB 11.2.

Simon


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-01-26 18:18 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-14 16:38 [PATCH v2] gdb: split array and string limiting options Maciej W. Rozycki
2021-12-14 17:24 ` Eli Zaretskii
2021-12-15 18:01 ` Andrew Burgess
2022-01-04 23:15   ` Maciej W. Rozycki
2022-01-06  7:58 Simon Sobisch
2022-01-06  9:33 ` Andrew Burgess
2022-01-06 13:32   ` Simon Sobisch
2022-01-06 14:18     ` Andrew Burgess
2022-01-26 15:48     ` Maciej W. Rozycki
2022-01-26 16:52       ` Simon Sobisch
2022-01-26 18:18         ` Maciej W. Rozycki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).