public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2] gdb/DAP Few bug fixes & Evaluate Array Watch vars
@ 2023-06-22 13:02 Simon Farre
  2023-06-22 14:06 ` Tom Tromey
  0 siblings, 1 reply; 2+ messages in thread
From: Simon Farre @ 2023-06-22 13:02 UTC (permalink / raw)
  To: gdb-patches; +Cc: tom, Simon Farre

v2.

EvaluateResult does not need a name, just as what Tom pointed out in
previous review. It's only the *children* that need to be made sure that
their names are valid. An identifier for a variable, can't ever have an
integer as a name, anyhow (not as far as I am aware, no programming
languages allow for that).

Removed the f-strings and use str() instead as pointed out that
f-strings might not be supported fully.

v1.

This patch fixes a few bugs.

First of all, name of VariableReferences must always be of string type.
This patch makes sure that this is the case by formatting the name. If
(when) the name is an integer, this will cause clients to fail or throw
errors.

Fixes a bug in NoOpArrayPrinter that calculated children to be N, but
only ever retrieves N-1 children, which makes Python at some time later
(during fetch_children -> fetch_one_child(N) ) raise an exception (out
of list index) which makes the entire request go bad.

The result[self.result_name] also f-strings the printer.to_string()
value, because this can potentially be a LazyString (which is a Python
object, not a string) and is not serializable by json.dumps.
---
 gdb/python/lib/gdb/dap/varref.py | 2 +-
 gdb/python/lib/gdb/printing.py   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/varref.py b/gdb/python/lib/gdb/dap/varref.py
index 23f18d647c3..213151fd3d3 100644
--- a/gdb/python/lib/gdb/dap/varref.py
+++ b/gdb/python/lib/gdb/dap/varref.py
@@ -66,7 +66,7 @@ class BaseReference:
             "variablesReference": self.ref,
         }
         if self.name is not None:
-            result["name"] = self.name
+            result["name"] = str(self.name)
         return result
 
     def no_children(self):
diff --git a/gdb/python/lib/gdb/printing.py b/gdb/python/lib/gdb/printing.py
index ab62ef3e45c..353427d000a 100644
--- a/gdb/python/lib/gdb/printing.py
+++ b/gdb/python/lib/gdb/printing.py
@@ -298,7 +298,7 @@ class NoOpArrayPrinter:
         return "array"
 
     def children(self):
-        for i in range(self.low, self.high):
+        for i in range(self.low, self.high + 1):
             yield (i, self.value[i])
 
 
-- 
2.41.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] gdb/DAP Few bug fixes & Evaluate Array Watch vars
  2023-06-22 13:02 [PATCH v2] gdb/DAP Few bug fixes & Evaluate Array Watch vars Simon Farre
@ 2023-06-22 14:06 ` Tom Tromey
  0 siblings, 0 replies; 2+ messages in thread
From: Tom Tromey @ 2023-06-22 14:06 UTC (permalink / raw)
  To: Simon Farre via Gdb-patches; +Cc: Simon Farre, tom

>>>>> "Simon" == Simon Farre via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> EvaluateResult does not need a name, just as what Tom pointed out in
Simon> previous review. It's only the *children* that need to be made sure that
Simon> their names are valid. An identifier for a variable, can't ever have an
Simon> integer as a name, anyhow (not as far as I am aware, no programming
Simon> languages allow for that).

FWIW Rust uses them as tuple field names, but in gdb I think they'd wind
up as string regardless.

Thanks for the patch, this is ok.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-06-22 14:06 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-22 13:02 [PATCH v2] gdb/DAP Few bug fixes & Evaluate Array Watch vars Simon Farre
2023-06-22 14:06 ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).