Re: [RFAv3 0/3] Convenience functions $_gdb_setting/$_gdb_int_setting

[Philippe Waroquiers <philippe.waroquiers@skynet.be>]

On Wed, 2019-07-10 at 16:34 +0100, Pedro Alves wrote:
> Alright, sounds good.  That's the sort of rationale that I'm looking
> for.
Ok, I will add the rational in the commit log.


> > > 
> > > Also, it seems like a design issue that settings that are unsigned
> > > internally get their values exposed as signed.
> > 
> > Not sure I understand the comment.
> > $_gdb_int_setting returns an integer value that is the same as what
> > the user can type.  If the user can type -1 (as a synonym for unlimited),
> > then $_gdb_int_setting returns -1, otherwise it will show 0 for unlimited.
> 
> For example:
> 
> > +    case var_uinteger:
> > +      if (*(unsigned int *) cmd->var == UINT_MAX)
> > +	return value_from_longest (builtin_type (gdbarch)->builtin_int,
> > +				   0);
> > +      else
> > +	return value_from_longest (builtin_type (gdbarch)->builtin_int,
> > +				   *(unsigned int *) cmd->var);
> 
> Why is this value_from_longest + builtin_int instead of
> value_from_ulongest + builtin_unsigned_int ?
I understand now.  Effectively, that is bad.
And the mapping of auto-boolean is worse.
I will fix all this.


> I think that $_gdb_int_setting should use the same convention
> as the "set" command.  I.e., conceptually, this should leave
> the setting unmodified:
> 
>  (gdb) set breakpoint pending $_gdb_int_setting("breakpoint pending")
Yes, that should have been the idea.

> 
> Should we normalize that throughout by changing auto_boolean to this:
> 
>  enum auto_boolean
>  {
>    AUTO_BOOLEAN_TRUE = 1
>    AUTO_BOOLEAN_FALSE = 0,
>    AUTO_BOOLEAN_AUTO = -1
>  };
> 
> ?
Doing this change will make the conversion of auto-boolean to int
a simple conversion (like boolean), rather than doing it with a
switch.
I guess that apart of making the conversion code simpler,
it will have no effect if all the code is properly doing comparisons
to AUTO_BOOLEAN_TRUE/FALSE/AUTO.


> I wonder whether gdb.parameter's handling of auto-booleans is
> also mismatched.

Here is an extract of the documentation:
   -- Function: gdb.parameter (parameter)
     ... Otherwise, the
     parameter's value is converted to a Python value of the appropriate
     type, and returned.

So, what is an 'appropriate type' for an auto-boolean ?

Doing the test:
  (gdb) set breakpoint pending 0
  (gdb) python print(gdb.parameter("breakpoint pending"))
  False
  (gdb) set breakpoint pending 1
  (gdb) python print(gdb.parameter("breakpoint pending"))
  True
  (gdb) set breakpoint pending -1
  (gdb) python print(gdb.parameter("breakpoint pending"))
  None
  (gdb) 

So, the above looks like a reasonable mapping for auto-boolean,
as soon as you understand that None is auto.

This mapping is described in python.texi, in the section
telling how to create new parameters.

However, the behaviour seems not fully consistent for all
integer settings:
  (gdb) set height 0
  (gdb) show height
  Number of lines gdb thinks are in a page is unlimited.
  (gdb) python print(gdb.parameter("height"))
  None
  (gdb) 

but ...
  (gdb) set history size unlimited
  (gdb) python print(gdb.parameter("history size"))
  -1
  (gdb) 

So, not too logical here ...

(the doc seems to not describe None being unlimited).


> 
> On the testing side, would it be possible to convert the testing
> to use the "maint set/show test-settings" subcommands, and thus
> be able to test all the different kinds of settings, in the
> same spirit as gdb.base/settings.exp and gdb.base/with.exp? 
> 
> I think the only impediment is accessing the maint settings,
> which you could do by adding a $_gdb_maint_setting function,
> much like I had added "maint with" for testing the "with"
> command's infrastructure.  It's bound to be useful anyway,
> so that scripts can access the main settings.
Good ideas ...


Thanks for all the comments, I will prepare a new version ...

Philippe