Re: [PATCH] gdb: split array and string limiting options

["Maciej W. Rozycki" <macro@embecosm.com>]

On Wed, 8 Dec 2021, Simon Marchi wrote:

> > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> > index a6f207a41a7..a92727bb3f1 100644
> > --- a/gdb/doc/gdb.texinfo
> > +++ b/gdb/doc/gdb.texinfo
> > @@ -9994,10 +9994,15 @@
> >  Set printing of array indexes.
> >  Related setting: @ref{set print array-indexes}.
> >  
> > +@item -characters @var{number-of-characters}|@code{unlimited}
> > +Set limit on string character to print.  The value @code{unlimited}
> 
> character -> characters?

 Sure!

> > diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
> > index d45df5fd113..98b5cceed3f 100644
> > --- a/gdb/python/py-value.c
> > +++ b/gdb/python/py-value.c
> > @@ -623,6 +623,7 @@ valpy_format_string (PyObject *self, PyObject *args, PyObject *kw)
> >        "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 elements N.  */
> 
> I guess the comment should say "see set print characters N".

 Fixed, obvious.

> We would need a corresponding doc update for this, in python.texi.

 Right, completed in the obvious way.

> > diff --git a/gdb/valprint.h b/gdb/valprint.h
> > index e1dae2cc8fc..f58d6ba7b0e 100644
> > --- a/gdb/valprint.h
> > +++ b/gdb/valprint.h
> > @@ -54,6 +54,9 @@ struct value_print_options
> >       "unlimited".  */
> >    unsigned int print_max;
> >  
> > +  /* Maxiumum number of string chars to print */
> 
> Maxiumum -> Maximum.  End with a period + 2 spaces.

 Fixed, and actually expanded, taking from the preceding comment which has 
had to be adjusted accordingly.

> > +  unsigned int print_smax;
> 
> I would suggest using a clearer name, like print_max_chars.

 Suggestion accepted as obvious and applied throughout.  This has resulted 
in some code and comment reformatting as needed, fixing preexisting style 
issues in the adjacent lines where appropriate too.

> Although Eli raised a good question, do we limit the number bytes of
> characters?  What would happen if we print a wide char or UTF-8 string?

 AFAICT the intent is to count actual characters however they are 
represented rather than bytes, e.g. `printstr' in ada-valprint.c has:

  for (i = 0; i < length && things_printed < options->print_max_chars; i += 1)
    {
[...]
	  ada_emit_char (char_at (string, i, type_len, byte_order),
			 elttype, stream, '"', type_len);

and `char_at' does interpret multibyte characters correctly:

/* Character #I of STRING, given that TYPE_LEN is the size in bytes
   of a character.  */

and similarly `generic_printstr' in valprint.c has:

  int width = TYPE_LENGTH (type);
[...]
  /* Arrange to iterate over the characters, in wchar_t form.  */
  wchar_iterator iter (string, length * width, encoding, width);

etc.  I guess there may be bugs somewhere, but this change does not affect 
them and is not supposed to, as it only splits a preexisting setting into 
two while retaining the original interpretation.  So for all intents and 
purposes I think it is appropriate to refer to string characters rather 
string bytes here.  Does this investigation clear your concerns?

 Thank you for your review.  Posting v2 separately.

  Maciej