[RFA] Handle var_zuinteger and var_zuinteger_unlimited from Python
Commit Message
PR python/20084 points out that the Python API doesn't handle the
var_zuinteger and var_zuinteger_unlimited parameter types.
This patch adds support for these types.
Regression tested on x86-64 Fedora 26.
gdb/ChangeLog
2018-04-26 Tom Tromey <tom@tromey.com>
PR python/20084:
* python/python.c (gdbpy_parameter_value): Handle var_zuinteger
and var_zuinteger_unlimited.
* python/py-param.c (struct parm_constant): Add PARAM_ZUINTEGER
and PARAM_ZUINTEGER_UNLIMITED.
(set_parameter_value): Handle var_zuinteger and
var_zuinteger_unlimited.
(add_setshow_generic): Likewise.
(parmpy_init): Likewise.
gdb/doc/ChangeLog
2018-04-26 Tom Tromey <tom@tromey.com>
PR python/20084:
* python.texi (Parameters In Python): Document PARAM_ZUINTEGER and
PARAM_ZUINTEGER_UNLIMITED.
gdb/testsuite/ChangeLog
2018-04-26 Tom Tromey <tom@tromey.com>
PR python/20084:
* gdb.python/py-parameter.exp: Add PARAM_ZUINTEGER and
PARAM_ZUINTEGER_UNLIMITED tests.
---
gdb/ChangeLog | 12 +++++++
gdb/doc/ChangeLog | 6 ++++
gdb/doc/python.texi | 14 ++++++++
gdb/python/py-param.c | 54 +++++++++++++++++++++++++------
gdb/python/python.c | 7 ++++
gdb/testsuite/ChangeLog | 6 ++++
gdb/testsuite/gdb.python/py-parameter.exp | 21 ++++++++++++
7 files changed, 111 insertions(+), 9 deletions(-)
Comments
> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>
> Date: Thu, 26 Apr 2018 16:20:03 -0600
>
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index ebd48fffe7..ca9114864b 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -3800,6 +3800,20 @@ The value is a filename. This is just like
> The value is an integer. This is like @code{PARAM_INTEGER}, except 0
> is interpreted as itself.
>
> +@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.
> +
> +@findex PARAM_ZUINTEGER_UNLIMITED
> +@findex gdb.PARAM_ZUINTEGER_UNLIMITED
> +@item gdb.PARAM_ZUINTEGER_UNLIMITED
> +The value is an unsigned integer. This is like @code{PARAM_ZUINTEGER},
> +except 0 is interpreted as itself, and the special value -1 should be
> +interpreted to mean ``unlimited''. Other negative values are not
> +allowed.
> +
This part is approved, but it sounds like only 1 of the 3 differences
from PARAM_ZUINTEGER should actually be mentioned, as the other 2 --
that 0 is interpreted as itself and negative values are generally not
allowed -- are already present in PARAM_ZUINTEGER. Or did I miss
something?
Thanks.
Hi Tom,
> PR python/20084 points out that the Python API doesn't handle the
> var_zuinteger and var_zuinteger_unlimited parameter types.
>
> This patch adds support for these types.
>
> Regression tested on x86-64 Fedora 26.
>
> gdb/ChangeLog
> 2018-04-26 Tom Tromey <tom@tromey.com>
>
> PR python/20084:
> * python/python.c (gdbpy_parameter_value): Handle var_zuinteger
> and var_zuinteger_unlimited.
> * python/py-param.c (struct parm_constant): Add PARAM_ZUINTEGER
> and PARAM_ZUINTEGER_UNLIMITED.
> (set_parameter_value): Handle var_zuinteger and
> var_zuinteger_unlimited.
> (add_setshow_generic): Likewise.
> (parmpy_init): Likewise.
>
> gdb/doc/ChangeLog
> 2018-04-26 Tom Tromey <tom@tromey.com>
>
> PR python/20084:
> * python.texi (Parameters In Python): Document PARAM_ZUINTEGER and
> PARAM_ZUINTEGER_UNLIMITED.
>
> gdb/testsuite/ChangeLog
> 2018-04-26 Tom Tromey <tom@tromey.com>
>
> PR python/20084:
> * gdb.python/py-parameter.exp: Add PARAM_ZUINTEGER and
> PARAM_ZUINTEGER_UNLIMITED tests.
This patch looks good to me, and you can push as is. It was surprisingly
harder to read (I picked this patch as a way to spend my time constructively,
while I wait for a build to finish ;-)), in particular the part with
the fall through in the case statement, but that's probably me having
a slightly different type of brain... I managed to convince myself
that the patch looks correct to me.
One thought: How about testing the value of the setting after setting
its value to -1?
>> +@findex PARAM_ZUINTEGER_UNLIMITED
>> +@findex gdb.PARAM_ZUINTEGER_UNLIMITED
>> +@item gdb.PARAM_ZUINTEGER_UNLIMITED
>> +The value is an unsigned integer. This is like @code{PARAM_ZUINTEGER},
>> +except 0 is interpreted as itself, and the special value -1 should be
>> +interpreted to mean ``unlimited''. Other negative values are not
>> +allowed.
>> +
Eli> This part is approved, but it sounds like only 1 of the 3 differences
Eli> from PARAM_ZUINTEGER should actually be mentioned, as the other 2 --
Eli> that 0 is interpreted as itself and negative values are generally not
Eli> allowed -- are already present in PARAM_ZUINTEGER. Or did I miss
Eli> something?
No, you didn't, that seems correct to me.
Also the value is actually signed.
I've changed it to:
The value is an 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.
Tom
>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
Joel> This patch looks good to me, and you can push as is. It was surprisingly
Joel> harder to read (I picked this patch as a way to spend my time constructively,
Joel> while I wait for a build to finish ;-)), in particular the part with
Joel> the fall through in the case statement, but that's probably me having
Joel> a slightly different type of brain... I managed to convince myself
Joel> that the patch looks correct to me.
Yeah, that bit is really unclear in the patch, but clearer (IMO) in the
code:
case var_uinteger:
if (l == 0)
l = UINT_MAX;
/* Fall through. */
case var_zuinteger:
ok = (l >= 0 && l <= UINT_MAX);
break;
Joel> One thought: How about testing the value of the setting after setting
Joel> its value to -1?
I've added a test like so:
gdb_test "python print(gdb.parameter('test-$kind'))" "-1" \
"check that PARAM_ZUINTEGER value is -1 after setting"
Tom
Eli> This part is approved, but it sounds like only 1 of the 3 differences
Eli> from PARAM_ZUINTEGER should actually be mentioned, as the other 2 --
Eli> that 0 is interpreted as itself and negative values are generally not
Eli> allowed -- are already present in PARAM_ZUINTEGER. Or did I miss
Eli> something?
Tom> No, you didn't, that seems correct to me.
Tom> Also the value is actually signed.
Tom> I've changed it to:
Tom> The value is an signed integer. This is like @code{PARAM_ZUINTEGER},
Tom> except the special value -1 should be interpreted to mean
Tom> ``unlimited''. Other negative values are not allowed.
So what I actually wrote and what you wrote that I agreed with are not
the same: I left in the text about other negative values. I thought
this still made sense given that the value is actually signed, but let
me know which way you'd prefer.
thanks,
Tom
> Yeah, that bit is really unclear in the patch, but clearer (IMO) in the
> code:
>
> case var_uinteger:
> if (l == 0)
> l = UINT_MAX;
> /* Fall through. */
> case var_zuinteger:
> ok = (l >= 0 && l <= UINT_MAX);
> break;
That's also how I ended up reviewing the patch :).
> Joel> One thought: How about testing the value of the setting after setting
> Joel> its value to -1?
>
> I've added a test like so:
>
> gdb_test "python print(gdb.parameter('test-$kind'))" "-1" \
> "check that PARAM_ZUINTEGER value is -1 after setting"
Looks good!
Thanks Tom,
> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>, gdb-patches@sourceware.org
> Date: Wed, 02 May 2018 10:01:34 -0600
>
> The value is an signed integer. This is like @code{PARAM_ZUINTEGER},
^^^^^^^^^^^^^^^^^
"a signed integer"?
> except the special value -1 should be interpreted to mean
> ``unlimited''. Other negative values are not allowed.
Thanks, sounds good.
>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
>> From: Tom Tromey <tom@tromey.com>
>> Cc: Tom Tromey <tom@tromey.com>, gdb-patches@sourceware.org
>> Date: Wed, 02 May 2018 10:01:34 -0600
>>
>> The value is an signed integer. This is like @code{PARAM_ZUINTEGER},
Eli> ^^^^^^^^^^^^^^^^^
Eli> "a signed integer"?
Thanks, I've fixed this.
Tom
On 04/26/2018 11:20 PM, Tom Tromey wrote:
> + if {$kind == "PARAM_ZUINTEGER"} {
> + gdb_test "python test_param_$kind.value = -1" "RuntimeError: Range exceeded.*"
> + } else {
> + gdb_test_no_output "python test_param_$kind.value = -1" ""
Passing "" to gdb_test_no_output case suppresses the test
name/message in gdb.sum. I suspect that was not intended.
Thanks,
Pedro Alves
@@ -3800,6 +3800,20 @@ The value is a filename. This is just like
The value is an integer. This is like @code{PARAM_INTEGER}, except 0
is interpreted as itself.
+@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.
+
+@findex PARAM_ZUINTEGER_UNLIMITED
+@findex gdb.PARAM_ZUINTEGER_UNLIMITED
+@item gdb.PARAM_ZUINTEGER_UNLIMITED
+The value is an unsigned integer. This is like @code{PARAM_ZUINTEGER},
+except 0 is interpreted as itself, and the special value -1 should be
+interpreted to mean ``unlimited''. Other negative values are not
+allowed.
+
@findex PARAM_ENUM
@findex gdb.PARAM_ENUM
@item gdb.PARAM_ENUM
@@ -47,6 +47,8 @@ struct parm_constant parm_constants[] =
{ "PARAM_OPTIONAL_FILENAME", var_optional_filename },
{ "PARAM_FILENAME", var_filename },
{ "PARAM_ZINTEGER", var_zinteger },
+ { "PARAM_ZUINTEGER", var_zuinteger },
+ { "PARAM_ZUINTEGER_UNLIMITED", var_zuinteger_unlimited },
{ "PARAM_ENUM", var_enum },
{ NULL, 0 }
};
@@ -225,6 +227,8 @@ set_parameter_value (parmpy_object *self, PyObject *value)
case var_integer:
case var_zinteger:
case var_uinteger:
+ case var_zuinteger:
+ case var_zuinteger_unlimited:
{
long l;
int ok;
@@ -239,20 +243,33 @@ set_parameter_value (parmpy_object *self, PyObject *value)
if (! gdb_py_int_as_long (value, &l))
return -1;
- if (self->type == var_uinteger)
+ switch (self->type)
{
- ok = (l >= 0 && l <= UINT_MAX);
+ case var_uinteger:
if (l == 0)
l = UINT_MAX;
- }
- else if (self->type == var_integer)
- {
+ /* Fall through. */
+ case var_zuinteger:
+ ok = (l >= 0 && l <= UINT_MAX);
+ break;
+
+ case var_zuinteger_unlimited:
+ ok = (l >= -1 && l <= INT_MAX);
+ break;
+
+ case var_integer:
ok = (l >= INT_MIN && l <= INT_MAX);
if (l == 0)
l = INT_MAX;
+ break;
+
+ case var_zinteger:
+ ok = (l >= INT_MIN && l <= INT_MAX);
+ break;
+
+ default:
+ gdb_assert_not_reached ("unknown var_ constant");
}
- else
- ok = (l >= INT_MIN && l <= INT_MAX);
if (! ok)
{
@@ -261,7 +278,10 @@ set_parameter_value (parmpy_object *self, PyObject *value)
return -1;
}
- self->value.intval = (int) l;
+ if (self->type == var_uinteger || self->type == var_zuinteger)
+ self->value.uintval = (unsigned) l;
+ else
+ self->value.intval = (int) l;
break;
}
@@ -526,6 +546,21 @@ add_setshow_generic (int parmclass, enum command_class cmdclass,
set_list, show_list);
break;
+ case var_zuinteger:
+ add_setshow_zuinteger_cmd (cmd_name, cmdclass,
+ &self->value.uintval, set_doc, show_doc,
+ help_doc, get_set_value, get_show_value,
+ set_list, show_list);
+ break;
+
+ case var_zuinteger_unlimited:
+ add_setshow_zuinteger_unlimited_cmd (cmd_name, cmdclass,
+ &self->value.intval, set_doc,
+ show_doc, help_doc, get_set_value,
+ get_show_value,
+ set_list, show_list);
+ break;
+
case var_enum:
add_setshow_enum_cmd (cmd_name, cmdclass, self->enumeration,
&self->value.cstringval, set_doc, show_doc,
@@ -658,7 +693,8 @@ parmpy_init (PyObject *self, PyObject *args, PyObject *kwds)
&& parmclass != var_uinteger && parmclass != var_integer
&& parmclass != var_string && parmclass != var_string_noescape
&& parmclass != var_optional_filename && parmclass != var_filename
- && parmclass != var_zinteger && parmclass != var_enum)
+ && parmclass != var_zinteger && parmclass != var_zuinteger
+ && parmclass != var_zuinteger_unlimited && parmclass != var_enum)
{
PyErr_SetString (PyExc_RuntimeError,
_("Invalid parameter class argument."));
@@ -467,6 +467,7 @@ gdbpy_parameter_value (enum var_types type, void *var)
Py_RETURN_NONE;
/* Fall through. */
case var_zinteger:
+ case var_zuinteger_unlimited:
return PyLong_FromLong (* (int *) var);
case var_uinteger:
@@ -477,6 +478,12 @@ gdbpy_parameter_value (enum var_types type, void *var)
Py_RETURN_NONE;
return PyLong_FromUnsignedLong (val);
}
+
+ case var_zuinteger:
+ {
+ unsigned int val = * (unsigned int *) var;
+ return PyLong_FromUnsignedLong (val);
+ }
}
return PyErr_Format (PyExc_RuntimeError,
@@ -179,3 +179,24 @@ gdb_test "python print (test_param.value)" "False" "test parameter value"
gdb_test "help show print test-param" "State of the Test Parameter.*" "test show help"
gdb_test "help set print test-param" "Set the state of the Test Parameter.*" "test set help"
gdb_test "help set print" "set print test-param -- Set the state of the Test Parameter.*" "test general help"
+
+foreach kind {PARAM_ZUINTEGER PARAM_ZUINTEGER_UNLIMITED} {
+ gdb_py_test_multiple "Simple gdb $kind" \
+ "python" "" \
+ "class TestNodocParam (gdb.Parameter):" "" \
+ " def __init__ (self, name):" "" \
+ " super (TestNodocParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.$kind)" "" \
+ " self.value = 0" "" \
+ "test_param_$kind = TestNodocParam ('test-$kind')" "" \
+ "end"
+
+ gdb_test "python print(gdb.parameter('test-$kind'))" "0"
+
+ gdb_test "python test_param_$kind.value = -5" "RuntimeError: Range exceeded.*"
+
+ if {$kind == "PARAM_ZUINTEGER"} {
+ gdb_test "python test_param_$kind.value = -1" "RuntimeError: Range exceeded.*"
+ } else {
+ gdb_test_no_output "python test_param_$kind.value = -1" ""
+ }
+}