Re: [PATCH 1/2] GDB: S12Z: Add assertion

John Darrington
2018-11-18T09:23:07Z

On Sat, Nov 17, 2018 at 02:59:28PM -0700, Kevin Buettner wrote:

> ---
> gdb/s12z-tdep.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/gdb/s12z-tdep.c b/gdb/s12z-tdep.c
> index 79f5772035..bd0bd7c001 100644
> --- a/gdb/s12z-tdep.c
> +++ b/gdb/s12z-tdep.c
> @@ -320,6 +320,7 @@ s12z_frame_cache (struct frame_info *this_frame, void **prologue_cache)
> }
> else
> {
> + gdb_assert (this_sp == this_sp_for_id);
> /* The stack pointer of the prev frame is frame_size greater
> than the stack pointer of this frame plus one address
> size (caused by the JSR or BSR). */
> --

Could you please add a ChangeLog entry?

Also, on the same topic, what happened to the ChangeLog entries
for the initial commit / push of the s12z port? (I don't see them
in the ChangeLog file.)

Is it really necessary in 2018 to have ChangeLogs? After all, the same
information is available by typing git log and/or git diff. If the answer is
"yes", then how about generating it automatically using the git-log-to-changelog
script which is available in gnulib ? Many other projects have gone over to
this method.

ChangeLogs are a dreaded nuisance. They are almost guaranteed to cause merge
conflicts. The information they contain is redundant. Their formatting
requirements are onerous. And in thirty years of software development, I don't
think I've ever experienced a case where I've needed to refer to one.

As for adding the assert, I suppose that you're just reminding the
reader of this earlier assignment? ...

this_sp_for_id = this_sp;

Indeed.

(I didn't see anything which might change either this_sp or
this_sp_for_id along the code path leading to the assert.)

I have some plans for future changes in this part of the code.

J'

--
Avoid eavesdropping. Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3
fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.