This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch][python] PR python/16324 (frame_id_eq check invalid)
- From: Pedro Alves <palves at redhat dot com>
- To: Phil Muldoon <pmuldoon at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 26 Feb 2014 11:12:22 +0000
- Subject: Re: [patch][python] PR python/16324 (frame_id_eq check invalid)
- Authentication-results: sourceware.org; auth=none
- References: <530DAA99 dot 6090209 at redhat dot com>
On 02/26/2014 08:49 AM, Phil Muldoon wrote:
> +
> + /* If prev_frame != NULL, and get_frame_id cannot return
> + the frame_id, there is nothing more to be done. At
> + this point, assert that a valid frame id is
> + returned. */
A nit (sorry!) - I find this "there is nothing more to be
done" in the comment a little bit confusing/distracting. That
reads to me like a comment we'd write when doing a regular
early return, because whatever the function was supposed to
had already been done.
You had a good rationale in the intro. I'd suggest basing on it,
saying this instead:
/* All frames other than the sentinel (which PREV_FRAME can't
be, as we are traversing the opposite direction)
must have a valid id. If we get back a null_frame_id with
prev_frame != NULL, something is messed up. */
> + gdb_assert (frame_id_p (frame_id) != 0);
Please write:
gdb_assert (frame_id_p (frame_id));
The '_p' suffix stands for predicate, indicating the function
returns a boolean, and for those, we use 'if (foo())' / 'if (!foo())'.
OK with that change.
Thanks,
--
Pedro Alves