This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/3 v4] Process record support for PowerPC
- From: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- To: cole945 at gmail dot com (Wei-cheng Wang)
- Cc: brobecker at adacore dot com (Joel Brobecker), gdb-patches at sourceware dot org
- Date: Mon, 19 Jan 2015 19:04:47 +0100 (CET)
- Subject: Re: [PATCH 2/3 v4] Process record support for PowerPC
- Authentication-results: sourceware.org; auth=none
Wei-cheng Wang wrote:
> 2015-01-18 Wei-cheng Wang <cole945@gmail.com>
>
> * rs6000-tdep.c (ppc_process_record_op4, ppc_process_record_op19,
> ppc_process_record_op31, ppc_process_record_op59,
> ppc_process_record_op60, ppc_process_record_op63,
> ppc_process_record): Fix -Wformat warning.
> * rs6000-tdep.c (rs6000_epilogue_frame_cache, ppc_process_record_op60):
> Remove unused variables.
This is OK. Please check this in to fix the reported build issues.
> 2015-01-18 Wei-cheng Wang <cole945@gmail.com>
>
> * ppc-linux-tdep.c (ppc_skip_trampoline_code,
> ppc_canonicalize_syscall, ppc_linux_syscall_record,
> ppc_linux_record_signal, ppc_init_linux_record_tdep): Add comments.
> * ppc64-tdep.c (ppc64_skip_trampoline_code): Likewise.
> * rs6000-tdep.c (rs6000_epilogue_frame_cache,
> rs6000_epilogue_frame_this_id, rs6000_epilogue_frame_prev_register,
> rs6000_epilogue_frame_sniffer, ppc_record_vsr, ppc_process_record_op4,
> ppc_process_record_op19, ppc_process_record_op31,
> ppc_process_record_op59, ppc_process_record_op60,
> ppc_process_record_op63): Likewise.
A couple of minor issues:
> +/* Record registers which might be clobbered during system call.
> + Return 0 if succeed. */
I think proper English usage would be "Return 0 on success." or
"Return 0 if sucessful." (Here and elsewhere in the patch.)
> -/* Initialize linux_record_tdep if not initialized yet. */
> +/* Initialize linux_record_tdep if not initialized yet.
> + WORDSIZE is 4 or 8 for 32- or 64-bit PowerPC Linux respectively.
> + Sizes of data structures are initilzied accordingly. */
Typo "initilzied".
Otherwise, this is OK as well.
Two other things as an aside:
- When committing a patch to git, please add the ChangeLog to the
git commit message as well. Have a look with "git log" at the
existing practice how to format the commit messages for GDB.
- Given that you've now got commit access, please also add your name
to the MAINTAINERS file under "Write After Approval"; see here for
a recent example:
https://sourceware.org/ml/gdb-patches/2015-01/msg00517.html
Thanks,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com