This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3 3/4] Deprecate frame_stop_reason_string.
- From: Pedro Alves <palves at redhat dot com>
- To: Andrew Burgess <aburgess at broadcom dot com>, gdb-patches at sourceware dot org
- Date: Thu, 29 May 2014 10:00:44 +0100
- Subject: Re: [PATCH v3 3/4] Deprecate frame_stop_reason_string.
- Authentication-results: sourceware.org; auth=none
- References: <533EC5B7 dot 6080600 at broadcom dot com> <1398855344-25278-1-git-send-email-aburgess at broadcom dot com> <1398855344-25278-4-git-send-email-aburgess at broadcom dot com> <53861C53 dot 5060402 at redhat dot com> <538670AE dot 3050305 at broadcom dot 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