[v7,1/4] GDB/Python: Use None for `var_zuinteger_unlimited' value set to `unlimited'

Message ID alpine.DEB.2.20.2210290013440.19931@tpp.orcam.me.uk
State Superseded
Headers
Series gdb: split array and string limiting options |

Commit Message

Maciej W. Rozycki Oct. 29, 2022, 1:53 p.m. UTC
  Consistently with parameters of the PARAM_UINTEGER and PARAM_INTEGER 
types, return the special value of `None' for a PARAM_ZUINTEGER_UNLIMITED 
parameter set to `unlimited', fixing an inconsistency introduced with 
commit 0489430a0e1a ("Handle var_zuinteger and var_zuinteger_unlimited 
from Python"); cf. PR python/20084.  Adjust the testsuite accordingly.

This makes all the three parameter types consistent with each other as 
far as the use of `None' is concerned, and similar to the Guile/Scheme 
interface where the `#:unlimited' keyword is likewise used.  We have a 
precedent already documented for a similar API correction:

 -- Function: gdb.breakpoints ()
     Return a sequence holding all of GDB's breakpoints.  *Note
     Breakpoints In Python::, for more information.  In GDB version 7.11
     and earlier, this function returned 'None' if there were no
     breakpoints.  This peculiarity was subsequently fixed, and now
     'gdb.breakpoints' returns an empty sequence in this case.

made in the past.

And then we have documentation not matching the interface as far as the 
value of `None' already returned for parameters of the PARAM_UINTEGER 
and PARAM_INTEGER types is concerned, and the case of an incorrect 
assertion for PARAM_UINTEGER and PARAM_ZUINTEGER_UNLIMITED parameters in 
the sibling Guile/Scheme module making such parameters unusable that has 
never been reported, both indicating that these features may indeed not 
be heavily used, and therefore that the risk from such an API change is 
likely lower than the long-term burden of the inconsistency.

And where the value read from a parameter is only used for presentation 
purposes, then such a change is expected to be transparent.
---
Changes from v6:

- Add a comma in the first sentence of the change description.

Changes from v5:

- Refer to Python parameter types in the change description rather than
  underlying GDB variable types in preparation for breaking the tight 
  coupling between the two later in this series.

- Document existing and updated semantics in the GDB manual.

- Update the testsuite adjustment to fit in the now expanded test case.

- Add a NEWS entry.

No change from v4.

New change in v4.
---
 gdb/NEWS                                  |    7 +++++++
 gdb/doc/python.texi                       |   30 +++++++++++++++++++-----------
 gdb/python/python.c                       |   10 +++++++++-
 gdb/testsuite/gdb.python/py-parameter.exp |   16 +++++-----------
 4 files changed, 40 insertions(+), 23 deletions(-)

gdb-python-var-zuinteger-unlimited-none.diff
  

Comments

Eli Zaretskii Oct. 29, 2022, 3:06 p.m. UTC | #1
> Date: Sat, 29 Oct 2022 14:53:18 +0100 (BST)
> From: "Maciej W. Rozycki" <macro@embecosm.com>
> Cc: Simon Sobisch <simonsobisch@web.de>, Tom Tromey <tom@tromey.com>
> 
> ---
>  gdb/NEWS                                  |    7 +++++++
>  gdb/doc/python.texi                       |   30 +++++++++++++++++++-----------
>  gdb/python/python.c                       |   10 +++++++++-
>  gdb/testsuite/gdb.python/py-parameter.exp |   16 +++++-----------
>  4 files changed, 40 insertions(+), 23 deletions(-)

Thanks, the documentation parts are approved.
  

Patch

Index: src/gdb/NEWS
===================================================================
--- src.orig/gdb/NEWS
+++ src/gdb/NEWS
@@ -216,6 +216,13 @@  GNU/Linux/CSKY (gdbserver) csky*-*linux*
      must start with a character in the set [a-zA-Z], every subsequent
      character of a window's name must be in the set [-_.a-zA-Z0-9].
 
+  ** Parameters of gdb.PARAM_ZUINTEGER_UNLIMITED type now return the
+     value of None for the 'unlimited' setting, consistently with
+     parameters of gdb.PARAM_UINTEGER and gdb.PARAM_INTEGER types.
+     Parameters of all the three types now accept the value of None
+     to mean 'unlimited'.  The use of internal integer representation
+     for the 'unlimited' setting is now deprecated.
+
 * New features in the GDB remote stub, GDBserver
 
   ** GDBserver is now supported on LoongArch GNU/Linux.
Index: src/gdb/doc/python.texi
===================================================================
--- src.orig/gdb/doc/python.texi
+++ src/gdb/doc/python.texi
@@ -4598,14 +4598,16 @@  Python, true and false are represented u
 @findex PARAM_UINTEGER
 @findex gdb.PARAM_UINTEGER
 @item gdb.PARAM_UINTEGER
-The value is an unsigned integer.  The value of 0 should be
-interpreted to mean ``unlimited''.
+The value is an unsigned integer.  The value of @code{None} should be
+interpreted to mean ``unlimited'', and the value of 0 is reserved and
+should not be used.
 
 @findex PARAM_INTEGER
 @findex gdb.PARAM_INTEGER
 @item gdb.PARAM_INTEGER
-The value is a signed integer.  The value of 0 should be interpreted
-to mean ``unlimited''.
+The value is a signed integer.  The value of @code{None} should be
+interpreted to mean ``unlimited'', and the value of 0 is reserved and
+should not be used.
 
 @findex PARAM_STRING
 @findex gdb.PARAM_STRING
@@ -4635,21 +4637,27 @@  The value is a filename.  This is just l
 @findex PARAM_ZINTEGER
 @findex gdb.PARAM_ZINTEGER
 @item gdb.PARAM_ZINTEGER
-The value is an integer.  This is like @code{PARAM_INTEGER}, except 0
-is interpreted as itself.
+The value is a signed integer.  This is like @code{PARAM_INTEGER},
+except that 0 is allowed and the value of @code{None} is not supported.
 
 @findex PARAM_ZUINTEGER
 @findex gdb.PARAM_ZUINTEGER
 @item gdb.PARAM_ZUINTEGER
-The value is an unsigned integer.  This is like @code{PARAM_INTEGER},
-except 0 is interpreted as itself, and the value cannot be negative.
+The value is an unsigned integer.  This is like @code{PARAM_UINTEGER},
+except that 0 is allowed and the value of @code{None} is not supported.
 
 @findex PARAM_ZUINTEGER_UNLIMITED
 @findex gdb.PARAM_ZUINTEGER_UNLIMITED
 @item gdb.PARAM_ZUINTEGER_UNLIMITED
-The value is a signed integer.  This is like @code{PARAM_ZUINTEGER},
-except the special value -1 should be interpreted to mean
-``unlimited''.  Other negative values are not allowed.
+The value is a signed integer.  This is like @code{PARAM_INTEGER}
+including that the value of @code{None} should be interpreted to mean
+``unlimited'', except that 0 is allowed, and the value cannot be negative.
+
+In GDB version 12 and earlier, a parameter of this type when read would
+return -1 rather than @code{None} for the setting of ``unlimited''.
+This peculiarity was subsequently fixed, for consistency with parameters
+of @code{PARAM_UINTEGER} and @code{PARAM_INTEGER} types, so that all the
+three types return the value of @code{None} for ``unlimited''.
 
 @findex PARAM_ENUM
 @findex gdb.PARAM_ENUM
Index: src/gdb/python/python.c
===================================================================
--- src.orig/gdb/python/python.c
+++ src/gdb/python/python.c
@@ -509,9 +509,17 @@  gdbpy_parameter_value (const setting &va
 	Py_RETURN_NONE;
       /* Fall through.  */
     case var_zinteger:
-    case var_zuinteger_unlimited:
       return gdb_py_object_from_longest (var.get<int> ()).release ();
 
+    case var_zuinteger_unlimited:
+      {
+	int val = var.get<int> ();
+
+	if (val == -1)
+	  Py_RETURN_NONE;
+	return gdb_py_object_from_longest (val).release ();
+      }
+
     case var_uinteger:
       {
 	unsigned int val = var.get<unsigned int> ();
Index: src/gdb/testsuite/gdb.python/py-parameter.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.python/py-parameter.exp
+++ src/gdb/testsuite/gdb.python/py-parameter.exp
@@ -346,22 +346,16 @@  proc_with_prefix test_gdb_parameter { }
 	    "listsize" {
 		set param_get_zero None
 		set param_get_minus_one -1
-		set param_get_none None
-		set param_get_unlimited None
 		set param_set_minus_one ""
 	    }
 	    "print elements" {
 		set param_get_zero None
 		set param_get_minus_one None
-		set param_get_none None
-		set param_get_unlimited None
 		set param_set_minus_one $param_range_error
 	    }
 	    "max-completions" {
 		set param_get_zero 0
-		set param_get_minus_one -1
-		set param_get_none -1
-		set param_get_unlimited -1
+		set param_get_minus_one None
 		set param_set_minus_one ""
 	    }
 	    default {
@@ -392,13 +386,13 @@  proc_with_prefix test_gdb_parameter { }
 	    "test set to None"
 
 	gdb_test "python print(gdb.parameter('$param'))" \
-	    $param_get_none "test value of None"
+	    None "test value of None"
 
 	gdb_test_no_output "python gdb.set_parameter('$param', 'unlimited')" \
 	    "test set to 'unlimited'"
 
 	gdb_test "python print(gdb.parameter('$param'))" \
-	    $param_get_unlimited "test value of 'unlimited'"
+	    None "test value of 'unlimited'"
     }
 
     clean_restart
@@ -468,9 +462,9 @@  proc_with_prefix test_integer_parameter
 	    }
 	    PARAM_ZUINTEGER_UNLIMITED {
 		set param_get_zero 0
-		set param_get_minus_one -1
+		set param_get_minus_one None
 		set param_get_minus_five 1
-		set param_get_none -1
+		set param_get_none None
 		set param_set_minus_one ""
 		set param_set_minus_five $param_range_error
 		set param_set_none ""