Re: [patch][python] PR python/16324 (frame_id_eq check invalid)

[Pedro Alves <palves@redhat.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