[pushed] gdb/python: exception trying to create empty array

[pushed] gdb/python: exception trying to create empty array

[Joel Brobecker]

Hello,

The following python command fails:

    (gdb) python print gdb.lookup_type('char').array(1, 0)
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
    ValueError: Array length must not be negative
    Error while executing Python code.

The above is trying to create an empty array, which is fairly command
in Ada.

gdb/ChangeLog:

        * python/py-type.c (typy_array_1): Do not raise negative-length
        exception if N2 is equal to N1 - 1.

gdb/testsuite/ChangeLog:

        * gdb.python/py-type.exp: Add a couple test about empty
        array creation, and negative-length array creation.

Tested on x86_64-linux, no regression.  Pushed a obvious.

Thank you,
-- 
Joel

PS: There is the same error in guile, and I will push a patch
momentarily.

---
 gdb/ChangeLog                        | 5 +++++
 gdb/python/py-type.c                 | 2 +-
 gdb/testsuite/ChangeLog              | 5 +++++
 gdb/testsuite/gdb.python/py-type.exp | 6 ++++++
 4 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b3e1263..a6211bf 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2015-01-06  Joel Brobecker  <brobecker@adacore.com>
+
+	* python/py-type.c (typy_array_1): Do not raise negative-length
+	exception if N2 is equal to N1 - 1.
+
 2015-01-03  Doug Evans  <xdje42@gmail.com>
 
 	* c-exp.y: Whitespace cleanup.
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 54fc30f..8e82c99 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -528,7 +528,7 @@ typy_array_1 (PyObject *self, PyObject *args, int is_vector)
       n1 = 0;
     }
 
-  if (n2 < n1)
+  if (n2 < n1 - 1)
     {
       PyErr_SetString (PyExc_ValueError,
 		       _("Array length must not be negative"));
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 374c285..7db0809 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2015-01-06  Joel Brobecker  <brobecker@adacore.com>
+
+	* gdb.python/py-type.exp: Add a couple test about empty
+	array creation, and negative-length array creation.
+
 2015-01-02  Doug Evans  <xdje42@gmail.com>
 
 	* gdb.cp/nsalias.exp: Fix output of external/declaration flags.
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index 90de68d..c4c8d9f 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -247,6 +247,12 @@ restart_gdb "${binfile}"
 # Skip all tests if Python scripting is not enabled.
 if { [skip_python_tests] } { continue }
 
+gdb_test "python print gdb.lookup_type('char').array(1, 0)" \
+    "char \\\[0\\\]"
+
+gdb_test "python print gdb.lookup_type('char').array(1, -1)" \
+    "Array length must not be negative.*"
+
 with_test_prefix "lang_c" {
     runto_bp "break to inspect struct and array."
     test_fields "c"
-- 
1.9.1

Re: [pushed] gdb/python: exception trying to create empty array

[Doug Evans]

On Tue, Jan 6, 2015 at 7:09 AM, Joel Brobecker <brobecker@adacore.com> wrote:
> Hello,
>
> The following python command fails:
>
>     (gdb) python print gdb.lookup_type('char').array(1, 0)
>     Traceback (most recent call last):
>       File "<string>", line 1, in <module>
>     ValueError: Array length must not be negative
>     Error while executing Python code.
>
> The above is trying to create an empty array, which is fairly command
> in Ada.
>
> gdb/ChangeLog:
>
>         * python/py-type.c (typy_array_1): Do not raise negative-length
>         exception if N2 is equal to N1 - 1.
>
> gdb/testsuite/ChangeLog:
>
>         * gdb.python/py-type.exp: Add a couple test about empty
>         array creation, and negative-length array creation.
>
> Tested on x86_64-linux, no regression.  Pushed a obvious.
> [...]
> diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
> index 54fc30f..8e82c99 100644
> --- a/gdb/python/py-type.c
> +++ b/gdb/python/py-type.c
> @@ -528,7 +528,7 @@ typy_array_1 (PyObject *self, PyObject *args, int is_vector)
>        n1 = 0;
>      }
>
> -  if (n2 < n1)
> +  if (n2 < n1 - 1)
>      {
>        PyErr_SetString (PyExc_ValueError,
>                        _("Array length must not be negative"));

Hi.

I think it might not be immediately obvious to the reader why the test
is "n2 < n1 - 1".
[E.g, there's no
Can you add a comment?

Thanks!

[fwiw, I don't want to introduce thoughts of what might or might not
be an obvious
patch. I don't have a problem with checking in something one thinks is obvious.
I just read code like this and scratch my head for a bit, and wish we had more
comments explaining the why of things. A one liner

  /* Note: An empty array has n2 == n1 - 1.  */

would help this poor reader a lot.

And while not a requirement of this patch,
lookup_array_range_type could use a comment explaining what the
correct way of specifying an empty array is.
[I'm assuming that's the correct way, I haven't actually tried it. :-)]

Re: [pushed] gdb/python: exception trying to create empty array

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 321 bytes --]

> I think it might not be immediately obvious to the reader why the test
> is "n2 < n1 - 1".
> [E.g, there's no
> Can you add a comment?

Sure. Attached is the patch I just pushed.

gdb/ChangeLog:

        * guile/scm-type.c (tyscm_array_1): Add comment.
        * python/py-type.c (typy_array_1): Add comment.

-- 
Joel

[-- Attachment #2: 0001-python-guile-Add-comment-beside-conditions-testing-e.patch --]
[-- Type: text/x-diff, Size: 1795 bytes --]

From e810d75b1c9bef779b29df9d2c609fd5891d5917 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Wed, 7 Jan 2015 07:34:29 +0400
Subject: [PATCH] [python,guile] Add comment beside conditions testing empty
 arrays.

gdb/ChangeLog:

        * guile/scm-type.c (tyscm_array_1): Add comment.
        * python/py-type.c (typy_array_1): Add comment.
---
 gdb/ChangeLog        | 5 +++++
 gdb/guile/scm-type.c | 2 +-
 gdb/python/py-type.c | 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8e3737d..0b63d34 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2015-01-07  Joel Brobecker  <brobecker@adacore.com>
+
+	* guile/scm-type.c (tyscm_array_1): Add comment.
+	* python/py-type.c (typy_array_1): Add comment.
+
 2015-01-06  Joel Brobecker  <brobecker@adacore.com>
 
 	* guile/scm-type.c (tyscm_array_1): Do not raise out-of-range
diff --git a/gdb/guile/scm-type.c b/gdb/guile/scm-type.c
index 4f46139..196b4a1 100644
--- a/gdb/guile/scm-type.c
+++ b/gdb/guile/scm-type.c
@@ -713,7 +713,7 @@ tyscm_array_1 (SCM self, SCM n1_scm, SCM n2_scm, int is_vector,
       n1 = 0;
     }
 
-  if (n2 < n1 - 1)
+  if (n2 < n1 - 1) /* Note: An empty array has n2 == n1 - 1.  */
     {
       gdbscm_out_of_range_error (func_name, SCM_ARG3,
 				 scm_cons (scm_from_long (n1),
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 8e82c99..bf92363 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -528,7 +528,7 @@ typy_array_1 (PyObject *self, PyObject *args, int is_vector)
       n1 = 0;
     }
 
-  if (n2 < n1 - 1)
+  if (n2 < n1 - 1) /* Note: An empty array has n2 == n1 - 1.  */
     {
       PyErr_SetString (PyExc_ValueError,
 		       _("Array length must not be negative"));
-- 
1.9.1