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

[Pedro Alves <palves@redhat.com>]

On 05/29/2014 12:26 AM, Andrew Burgess wrote:
> On 28/05/2014 6:26 PM, Pedro Alves wrote:
>> On 04/30/2014 11:55 AM, Andrew Burgess wrote:

>> 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?

Not yet, sorry.

> 	* frame.c (frame_stop_reason_string): Rename to ...
> 	(unwind_frame_stop_reason_string): this.

This still has "frame" in the name, and misses the "to".  Any reason for that?

The specific suggestion had a logic --

 convert "enum unwind_stop_reason" to "string" -> "unwind_stop_reason_to_string".

That is just like target_waitstatus_to_string and
target_xfer_status_to_string, for example.

Without even looking at the function's declaration, I can tell
that is converting the enum value.  While "unwind_frame_stop_reason_string"
without the "to" doesn't give me that impression, and is very much
confusable with the new frame_stop_reason_string.

> 	* 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.

...

> 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.  */

Please remove this comment hunk from this patch, so that the patch
remains independent, and so that it can go in immediately even if
further discuss patch #4.

-- 
Pedro Alves