Re: [PATCH 00/16] Add styling to the gdb CLI and TUI

[Simon Marchi <simark@simark.ca>]

On 2019-03-06 11:02 a.m., Eli Zaretskii wrote:
>> From: Tom Tromey <tom@tromey.com>
>> Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
>> Date: Mon, 04 Mar 2019 10:40:46 -0700
>>
>>>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
>>
>> Eli> What do you think about the idea to add a convenience variable that
>> Eli> would provide the GDB version?
>>
>> Seems reasonable to me.
> 
> How about the patch below?  Is it okay to go in?  (Note that I took
> this opportunity to clean up whitespace in top.c, I hope it's OK to do
> that as part of unrelated code changes.)

Ah, I noticed this because you changed the subject in your ping :).  This would
be very useful, especially that we can quite easily query this from MI as well
as Python.

> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index ac61e65..f2915d0 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,10 @@
> +2019-03-06  Eli Zaretskii  <eliz@gnu.org>
> +
> +	* NEWS: Announce $_gdb_version.
> +
> +	* top.c (init_gdb_version_var): New function.
> +	(gdb_init): Call init_gdb_version_var.
> +
>  2019-03-06  Tom Tromey  <tromey@adacore.com>
>  
>  	* remote-sim.c (gdbsim_target_open): Use result of
> diff --git a/gdb/NEWS b/gdb/NEWS
> index cc7c35c..260e6cc 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,12 @@
>  
>  *** Changes since GDB 8.3
>  
> +* New built-in convenience variable $_gdb_version provides the GDB
> +  version.  It is handy for conditionally using features available
> +  only in or since specific GDB versions, in scripts that should work
> +  error-free with many different versions, such as in system-wide init
> +  files.
> +
>  *** Changes in GDB 8.3
>  
>  * GDB and GDBserver now support access to additional registers on
> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> index 0380322..313a061 100644
> --- a/gdb/doc/ChangeLog
> +++ b/gdb/doc/ChangeLog
> @@ -1,3 +1,7 @@
> +2019-03-06  Eli Zaretskii  <eliz@gnu.org>
> +
> +	* gdb.texinfo (Convenience Vars): Document $_gdb_version.
> +
>  2019-03-05  Simon Marchi  <simon.marchi@efficios.com>
>  
>  	* python.texi (Values From Inferior): Change synopsys of the
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index f2028f8..9d15337 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -11197,7 +11197,7 @@
>  @vindex $_tlb@r{, convenience variable}
>  The variable @code{$_tlb} is automatically set when debugging
>  applications running on MS-Windows in native mode or connected to
> -gdbserver that supports the @code{qGetTIBAddr} request. 
> +gdbserver that supports the @code{qGetTIBAddr} request.
>  @xref{General Query Packets}.
>  This variable contains the address of the thread information block.

This looks like an unintended change.

> @@ -11211,6 +11211,17 @@
>  @item $_gthread
>  The global number of the current thread.  @xref{global thread numbers}.
>  
> +@item $_gdb_version
> +@vindex $_gdb_version@r{, convenience variable}
> +The version of the running @value{GDBN}.  The value is an integer
> +number that encodes the major and minor @value{GDBN} versions as
> +@w{@code{@var{major}*100 + @var{minor}}}, so, e.g., @value{GDBN}
> +version 9.10 will produce the value @code{910}.  Development snapshots
> +and pretest versions have their minor version incremented by one;
> +thus, @value{GDBN} pretest 9.11.90 will produce the value 912.  This
> +variable allows you to write scripts that work with different versions
> +of @value{GDBN} without errors caused by features unavailable in some
> +of those versions.
>  @end table

I know we plan to move to a version scheme where we don't have a "patch"
number (a third number), but just in case, maybe we could plan for it anyway
just in case it ever changes again in the future (I don't expect it will,
but we never know.

So something like MAJOR * 10000 + MINOR * 100 + PATCH * 100

Also, it means that in your example, 9.11.90 would produce 091190.  I think
it's better if we are able to distinguish 9.11.90 from 9.12.

Also, we should consider doing like Python does, and encode different numbers
in different bytes:

  https://docs.python.org/3/c-api/apiabiversion.html

So we could say ((MAJOR << 16) | (MINOR << 8) | PATCH), for example.  The
advantage with this is that it's easy to to isolate a particular number
using bitshifts and masks.  I know it would be possible as well in decimal
to isolate a particular number, but it's just more convenient in hex.

>  
>  @node Convenience Funs
> diff --git a/gdb/top.c b/gdb/top.c
> index 22e6f7e..97b349a 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -147,22 +147,22 @@ int server_command;
>  
>  /* Timeout limit for response from target.  */
>  
> -/* The default value has been changed many times over the years.  It 
> -   was originally 5 seconds.  But that was thought to be a long time 
> +/* The default value has been changed many times over the years.  It
> +   was originally 5 seconds.  But that was thought to be a long time
>     to sit and wait, so it was changed to 2 seconds.  That was thought
> -   to be plenty unless the connection was going through some terminal 
> +   to be plenty unless the connection was going through some terminal
>     server or multiplexer or other form of hairy serial connection.
>  
> -   In mid-1996, remote_timeout was moved from remote.c to top.c and 
> +   In mid-1996, remote_timeout was moved from remote.c to top.c and
>     it began being used in other remote-* targets.  It appears that the
>     default was changed to 20 seconds at that time, perhaps because the
>     Renesas E7000 ICE didn't always respond in a timely manner.
>  
>     But if 5 seconds is a long time to sit and wait for retransmissions,
> -   20 seconds is far worse.  This demonstrates the difficulty of using 
> +   20 seconds is far worse.  This demonstrates the difficulty of using
>     a single variable for all protocol timeouts.
>  
> -   As remote.c is used much more than remote-e7000.c, it was changed 
> +   As remote.c is used much more than remote-e7000.c, it was changed
>     back to 2 seconds in 1999.  */

Some more unintended changes?  There are more in the patch, I won't point
out all of them.

I have to go for now, I didn't look at the implementation yet.

Simon