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

Pedro Alves
2014-05-29T09:00:52Z

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