* [review] gdb: Convert frame_show_address to return a bool
2019-11-08 16:22 [review] gdb: Convert frame_show_address to return a bool Andrew Burgess (Code Review)
@ 2019-11-08 16:27 ` Simon Marchi (Code Review)
2019-11-08 16:28 ` Simon Marchi (Code Review)
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Simon Marchi (Code Review) @ 2019-11-08 16:27 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
Simon Marchi has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/541
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
This is to test the new comment notification emails, but while at it, this change LGTM.
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/541/1/gdb/stack.c
File gdb/stack.c:
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/541/1/gdb/stack.c@255
PS1, Line 255:
250 | static int last_displayed_line = 0;
251 | \f
252 |
253 | /* See stack.h. */
254 |
255 > bool
256 | frame_show_address (struct frame_info *frame,
257 | struct symtab_and_line sal)
258 | {
259 | /* If there is a line number, but no PC, then there is no location
260 | information associated with this sal. The only way that should
I think this is a really good change.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iaaa9ebd4ff6534db19c5329f1c604932c747bd7f
Gerrit-Change-Number: 541
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Fri, 08 Nov 2019 16:27:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 7+ messages in thread* [review] gdb: Convert frame_show_address to return a bool
2019-11-08 16:22 [review] gdb: Convert frame_show_address to return a bool Andrew Burgess (Code Review)
2019-11-08 16:27 ` Simon Marchi (Code Review)
@ 2019-11-08 16:28 ` Simon Marchi (Code Review)
2019-11-08 16:33 ` Simon Marchi (Code Review)
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Simon Marchi (Code Review) @ 2019-11-08 16:28 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
Simon Marchi has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/541
......................................................................
Patch Set 1:
(1 comment)
Damn, I had made a silly typo. Test again.
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/541/1/gdb/stack.c
File gdb/stack.c:
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/541/1/gdb/stack.c@255
PS1, Line 255:
250 | static int last_displayed_line = 0;
251 | \f
252 |
253 | /* See stack.h. */
254 |
255 > bool
256 | frame_show_address (struct frame_info *frame,
257 | struct symtab_and_line sal)
258 | {
259 | /* If there is a line number, but no PC, then there is no location
260 | information associated with this sal. The only way that should
I think this is a really good change (take 2).
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iaaa9ebd4ff6534db19c5329f1c604932c747bd7f
Gerrit-Change-Number: 541
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Fri, 08 Nov 2019 16:28:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 7+ messages in thread* [review] gdb: Convert frame_show_address to return a bool
2019-11-08 16:22 [review] gdb: Convert frame_show_address to return a bool Andrew Burgess (Code Review)
2019-11-08 16:27 ` Simon Marchi (Code Review)
2019-11-08 16:28 ` Simon Marchi (Code Review)
@ 2019-11-08 16:33 ` Simon Marchi (Code Review)
2019-11-08 16:38 ` Simon Marchi (Code Review)
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Simon Marchi (Code Review) @ 2019-11-08 16:33 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
Simon Marchi has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/541
......................................................................
Patch Set 1:
(1 comment)
Silly me, again. Now I had a typo in the template.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iaaa9ebd4ff6534db19c5329f1c604932c747bd7f
Gerrit-Change-Number: 541
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Fri, 08 Nov 2019 16:33:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 7+ messages in thread* [review] gdb: Convert frame_show_address to return a bool
2019-11-08 16:22 [review] gdb: Convert frame_show_address to return a bool Andrew Burgess (Code Review)
` (2 preceding siblings ...)
2019-11-08 16:33 ` Simon Marchi (Code Review)
@ 2019-11-08 16:38 ` Simon Marchi (Code Review)
2019-11-10 21:02 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-11-10 21:02 ` Sourceware to Gerrit sync (Code Review)
5 siblings, 0 replies; 7+ messages in thread
From: Simon Marchi (Code Review) @ 2019-11-08 16:38 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
Simon Marchi has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/541
......................................................................
Patch Set 1:
(1 comment)
Thanks Andrew for letting me use you as a guinea pig for this change :).
> --- gdb/stack.c
> +++ gdb/stack.c
> @@ -247,18 +247,18 @@ static int last_displayed_sal_valid = 0;
> static struct program_space *last_displayed_pspace = 0;
> static CORE_ADDR last_displayed_addr = 0;
> static struct symtab *last_displayed_symtab = 0;
> static int last_displayed_line = 0;
> \f
>
> /* See stack.h. */
>
> -int
> +bool
PS1, Line 255:
I think this is a really good change (take 5).
> frame_show_address (struct frame_info *frame,
> struct symtab_and_line sal)
> {
> /* If there is a line number, but no PC, then there is no location
> information associated with this sal. The only way that should
> happen is for the call sites of inlined functions (SAL comes from
> find_frame_sal). Otherwise, we would have some PC range if the
> SAL came from a line table. */
> if (sal.line != 0 && sal.pc == 0 && sal.end == 0)
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iaaa9ebd4ff6534db19c5329f1c604932c747bd7f
Gerrit-Change-Number: 541
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Fri, 08 Nov 2019 16:38:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 7+ messages in thread* [pushed] gdb: Convert frame_show_address to return a bool
2019-11-08 16:22 [review] gdb: Convert frame_show_address to return a bool Andrew Burgess (Code Review)
` (3 preceding siblings ...)
2019-11-08 16:38 ` Simon Marchi (Code Review)
@ 2019-11-10 21:02 ` Sourceware to Gerrit sync (Code Review)
2019-11-10 21:02 ` Sourceware to Gerrit sync (Code Review)
5 siblings, 0 replies; 7+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-11-10 21:02 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches; +Cc: Simon Marchi
Sourceware to Gerrit sync has submitted this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/541
......................................................................
gdb: Convert frame_show_address to return a bool
Just a clean up, should be no user visible changes after this commit.
gdb/ChangeLog:
* stack.c (frame_show_address): Convert return type to bool.
* stack.h (frame_show_address): Likewise, and update header
comment.
Change-Id: Iaaa9ebd4ff6534db19c5329f1c604932c747bd7f
---
M gdb/ChangeLog
M gdb/stack.c
M gdb/stack.h
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b9d5f21..158af82 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
2019-11-10 Andrew Burgess <andrew.burgess@embecosm.com>
+ * stack.c (frame_show_address): Convert return type to bool.
+ * stack.h (frame_show_address): Likewise, and update header
+ comment.
+
+2019-11-10 Andrew Burgess <andrew.burgess@embecosm.com>
+
* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add new file to the list.
* unittests/vec-utils-selftests.c: New file.
* gdbsupport/gdb_vecs.h (unordered_remove): Avoid self move assign.
diff --git a/gdb/stack.c b/gdb/stack.c
index fa057db..5af00c7 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -252,7 +252,7 @@
/* See stack.h. */
-int
+bool
frame_show_address (struct frame_info *frame,
struct symtab_and_line sal)
{
@@ -267,7 +267,7 @@
gdb_assert (inline_skipped_frames (inferior_thread ()) > 0);
else
gdb_assert (get_frame_type (get_next_frame (frame)) == INLINE_FRAME);
- return 0;
+ return false;
}
return get_frame_pc (frame) != sal.pc;
diff --git a/gdb/stack.h b/gdb/stack.h
index dda4fc9..28d2273 100644
--- a/gdb/stack.h
+++ b/gdb/stack.h
@@ -49,10 +49,10 @@
void get_user_print_what_frame_info (gdb::optional<enum print_what> *what);
-/* Return 1 if we should display the address in addition to the location,
+/* Return true if we should display the address in addition to the location,
because we are in the middle of a statement. */
-int frame_show_address (struct frame_info *frame, struct symtab_and_line sal);
+bool frame_show_address (struct frame_info *frame, struct symtab_and_line sal);
/* Get or set the last displayed symtab and line, which is, e.g. where we set a
* breakpoint when `break' is supplied with no arguments. */
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iaaa9ebd4ff6534db19c5329f1c604932c747bd7f
Gerrit-Change-Number: 541
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: merged
^ permalink raw reply [flat|nested] 7+ messages in thread* [pushed] gdb: Convert frame_show_address to return a bool
2019-11-08 16:22 [review] gdb: Convert frame_show_address to return a bool Andrew Burgess (Code Review)
` (4 preceding siblings ...)
2019-11-10 21:02 ` [pushed] " Sourceware to Gerrit sync (Code Review)
@ 2019-11-10 21:02 ` Sourceware to Gerrit sync (Code Review)
5 siblings, 0 replies; 7+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-11-10 21:02 UTC (permalink / raw)
To: Andrew Burgess, Simon Marchi, gdb-patches
The original change was created by Andrew Burgess.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/541
......................................................................
gdb: Convert frame_show_address to return a bool
Just a clean up, should be no user visible changes after this commit.
gdb/ChangeLog:
* stack.c (frame_show_address): Convert return type to bool.
* stack.h (frame_show_address): Likewise, and update header
comment.
Change-Id: Iaaa9ebd4ff6534db19c5329f1c604932c747bd7f
---
M gdb/ChangeLog
M gdb/stack.c
M gdb/stack.h
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b9d5f21..158af82 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
2019-11-10 Andrew Burgess <andrew.burgess@embecosm.com>
+ * stack.c (frame_show_address): Convert return type to bool.
+ * stack.h (frame_show_address): Likewise, and update header
+ comment.
+
+2019-11-10 Andrew Burgess <andrew.burgess@embecosm.com>
+
* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add new file to the list.
* unittests/vec-utils-selftests.c: New file.
* gdbsupport/gdb_vecs.h (unordered_remove): Avoid self move assign.
diff --git a/gdb/stack.c b/gdb/stack.c
index fa057db..5af00c7 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -252,7 +252,7 @@
/* See stack.h. */
-int
+bool
frame_show_address (struct frame_info *frame,
struct symtab_and_line sal)
{
@@ -267,7 +267,7 @@
gdb_assert (inline_skipped_frames (inferior_thread ()) > 0);
else
gdb_assert (get_frame_type (get_next_frame (frame)) == INLINE_FRAME);
- return 0;
+ return false;
}
return get_frame_pc (frame) != sal.pc;
diff --git a/gdb/stack.h b/gdb/stack.h
index dda4fc9..28d2273 100644
--- a/gdb/stack.h
+++ b/gdb/stack.h
@@ -49,10 +49,10 @@
void get_user_print_what_frame_info (gdb::optional<enum print_what> *what);
-/* Return 1 if we should display the address in addition to the location,
+/* Return true if we should display the address in addition to the location,
because we are in the middle of a statement. */
-int frame_show_address (struct frame_info *frame, struct symtab_and_line sal);
+bool frame_show_address (struct frame_info *frame, struct symtab_and_line sal);
/* Get or set the last displayed symtab and line, which is, e.g. where we set a
* breakpoint when `break' is supplied with no arguments. */
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Iaaa9ebd4ff6534db19c5329f1c604932c747bd7f
Gerrit-Change-Number: 541
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: newpatchset
^ permalink raw reply [flat|nested] 7+ messages in thread