Re: [PATCH v3 3/4] Deprecate frame_stop_reason_string.

[Andrew Burgess <aburgess at broadcom dot com>]

On 28/05/2014 6:26 PM, Pedro Alves wrote:
> On 04/30/2014 11:55 AM, Andrew Burgess wrote:
>> Patch #4 adds frame specific stop reason strings.  It is better to use that
>> string rather than the existing generic stop reason string.  This patch
>> renames frame_stop_reason_string to deprecated_frame_stop_reason_string and
>> updates all the call sites to use the deprecated name.
>>
>> Patch #4 adds the new frame specific stop reason, and some uses of the
>> deprecated funciton will be moved to this new API, however, in the python
>> and guile scripting API we expose a function that converts the stop reason
>> code into a string, without a frame, this will be harder to convert to the
> 
> The function still serves it's purpose of mapping the enum values
> to string equivalents.  I don't really see a need to mark it deprecated.

I guess not.  In my head I figured if you were going to show a string then
you'd always be better going through the new frame specific interface, as
any string it returns might be "more specific" to this frame, and strings
are always (that's my assumption / guess) just being displayed to the user
so more specific would be better.
For program control you'd still have the get_frame_unwind_stop_reason which
returns the enum value.
I wasn't sure where the old function would still serve a really useful
role, and I worried it would be used in places the frame specific version
would be better.

That said, I don't object to keeping it around.  It can always be removed
later if it turns out that it's not used.

> IMO, the real problem with its signature is the "frame_" in its name.
> The function converts an 'enum unwind_stop_reason' value to a string,
> so like other similar cases in the tree, how about renaming the function
> to match?  E.g.:
> 
> -const char *frame_stop_reason_string (enum unwind_stop_reason);
> +const char *unwind_stop_reason_to_string (enum unwind_stop_reason);

I've gone with this suggestion, updated patch below.  Is this OK to apply?

Thanks,
Andrew


gdb/ChangeLog:

	* frame.c (frame_stop_reason_string): Rename to ...
	(unwind_frame_stop_reason_string): this.
	* frame.h (frame_stop_reason_string): Rename to ...
	(unwind_frame_stop_reason_string): this.
        * stack.c (frame_info): Update call to frame_stop_reason_string.
	(backtrace_command_1): Likewise.
	* guile/scm-frame.c (gdbscm_unwind_stop_reason_string): Likewise.
	* python/py-frame.c (gdbpy_frame_stop_reason_string): Likewise.
---
 gdb/frame.c           | 2 +-
 gdb/frame.h           | 6 ++++--
 gdb/guile/scm-frame.c | 2 +-
 gdb/python/py-frame.c | 2 +-
 gdb/stack.c           | 4 ++--
 5 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/gdb/frame.c b/gdb/frame.c
index cbff25f..ee2b711 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2561,7 +2561,7 @@ get_frame_unwind_stop_reason (struct frame_info *frame)
 /* Return a string explaining REASON.  */
 
 const char *
-frame_stop_reason_string (enum unwind_stop_reason reason)
+unwind_frame_stop_reason_string (enum unwind_stop_reason reason)
 {
   switch (reason)
     {
diff --git a/gdb/frame.h b/gdb/frame.h
index ad03a0b..5acb2a2 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -501,9 +501,11 @@ enum unwind_stop_reason
 
 enum unwind_stop_reason get_frame_unwind_stop_reason (struct frame_info *);
 
-/* Translate a reason code to an informative string.  */
+/* Translate a reason code to an informative string.  This returns a
+   general string describing the stop reason, for a possibly frame
+   specific reason string, use frame_stop_reason_string.  */
 
-const char *frame_stop_reason_string (enum unwind_stop_reason);
+const char *unwind_frame_stop_reason_string (enum unwind_stop_reason);
 
 /* Unwind the stack frame so that the value of REGNUM, in the previous
    (up, older) frame is returned.  If VALUEP is NULL, don't
diff --git a/gdb/guile/scm-frame.c b/gdb/guile/scm-frame.c
index 8800923..0191a7a 100644
--- a/gdb/guile/scm-frame.c
+++ b/gdb/guile/scm-frame.c
@@ -933,7 +933,7 @@ gdbscm_unwind_stop_reason_string (SCM reason_scm)
   if (reason < UNWIND_FIRST || reason > UNWIND_LAST)
     scm_out_of_range (FUNC_NAME, reason_scm);
 
-  str = frame_stop_reason_string (reason);
+  str = unwind_frame_stop_reason_string (reason);
   return gdbscm_scm_from_c_string (str);
 }
 
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 8c80d39..ba17837 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -588,7 +588,7 @@ gdbpy_frame_stop_reason_string (PyObject *self, PyObject *args)
       return NULL;
     }
 
-  str = frame_stop_reason_string (reason);
+  str = unwind_frame_stop_reason_string (reason);
   return PyUnicode_Decode (str, strlen (str), host_charset (), NULL);
 }
 
diff --git a/gdb/stack.c b/gdb/stack.c
index 297ba32..a113a03 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1529,7 +1529,7 @@ frame_info (char *addr_exp, int from_tty)
       reason = get_frame_unwind_stop_reason (fi);
       if (reason != UNWIND_NO_REASON)
 	printf_filtered (_(" Outermost frame: %s\n"),
-			 frame_stop_reason_string (reason));
+			 unwind_frame_stop_reason_string (reason));
     }
   else if (get_frame_type (fi) == TAILCALL_FRAME)
     puts_filtered (" tail call frame");
@@ -1848,7 +1848,7 @@ backtrace_command_1 (char *count_exp, int show_locals, int no_filters,
 	  reason = get_frame_unwind_stop_reason (trailing);
 	  if (reason >= UNWIND_FIRST_ERROR)
 	    printf_filtered (_("Backtrace stopped: %s\n"),
-			     frame_stop_reason_string (reason));
+			     unwind_frame_stop_reason_string (reason));
 	}
     }
 }
-- 
1.8.1.3