diff mbox

[review] gdb: Convert frame_show_address to return a bool

Message ID gerrit.1573230144000.Iaaa9ebd4ff6534db19c5329f1c604932c747bd7f@gnutoolchain-gerrit.osci.io
State New, archived
Headers

Commit Message

Simon Marchi (Code Review) Nov. 8, 2019, 4:22 p.m. UTC 
  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(-)

Comments

Simon Marchi (Code Review) Nov. 8, 2019, 4:27 p.m. UTC | #1 
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 | 
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.
Simon Marchi (Code Review) Nov. 8, 2019, 4:28 p.m. UTC | #2 
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 | 
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).
Simon Marchi (Code Review) Nov. 8, 2019, 4:33 p.m. UTC | #3 
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.
Simon Marchi (Code Review) Nov. 8, 2019, 4:38 p.m. UTC | #4 
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;
>  
>  
>  /* 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)
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f5c8a76..312eed7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@ 
+2019-11-08  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-08  Tom Tromey  <tromey@adacore.com>
 
 	* top.c (read_command_file): Update.
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.  */