This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [Patch] Microblaze: Port of Linux gdbserver
- From: Michael Eager <eager at eagerm dot com>
- To: Ajit Kumar Agarwal <ajit dot kumar dot agarwal at xilinx dot com>, Michael Eager <eager at eagercon dot com>, Pedro Alves <palves at redhat dot com>, Joel Brobecker <brobecker at adacore dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>, Vinod Kathail <vinodk at xilinx dot com>, Vidhumouli Hunsigida <vidhum at xilinx dot com>, Nagaraju Mekala <nmekala at xilinx dot com>
- Date: Thu, 09 Oct 2014 16:42:15 -0700
- Subject: Re: [Patch] Microblaze: Port of Linux gdbserver
- Authentication-results: sourceware.org; auth=none
- References: <25de23b98e054fd291ea232d10f2800c at BN1BFFO11FD018 dot protection dot gbl> <5436B7D0 dot 9060004 at eagercon dot com> <d0a23ff7601a4dbcab91d5b680bc2e13 at BY2FFO11FD017 dot protection dot gbl>
On 10/09/14 11:54, Ajit Kumar Agarwal wrote:
To send the patches after incorporating the comments, Is there any other way of sending the patches without top post?
After you address comments, include the changelog at the end and attach
the patch (unless it is just a few lines). That way we can tell how
you responded to each comment.
+#define microblaze_breakpoint_len 4
Use CAPS for macros.
The MIPS and the ARM gdbserver code does not use the CAPS for the above macro defined.
Let's follow the GNU coding standard, even if some other targets haven't.
+ (*the_target->read_memory) (where, (unsigned char *) &insn, 4);
+
+ if (insn == microblaze_breakpoint)
Why use the explicit length rather than the macro you just defined?
Why not use sizeof (insn)?
To match up with the MIPS target and ARM target they have not used the macro defined. In the Mips 4 is used and in the ARM target for the THUMB_ARM 2 is used and for the ARM Mode code 4 is used.
Let's follow good coding practice, even if there have been lapses in the past.
Unless there is some particular relevance to instruction length on MIPS or
ARM/Thumb, let's stick to what is relevant to MicroBlaze.
Pedro:
I'd much prefer if we had that patch in the tree before accepting further patches that tweak things around register names, etc. Could you send that (as an independent patch, in a new thread).
Please address issues with previous patches before moving on to submit dependent patches.
I have already send the patch related to the above Pedro's comment. I have also send the patch after incorporating the Pedro feedback comments.
I haven't seen this patch. Please let me know when you posted it, or
send me a link to it in the mailing list archive.
If you submit a patch which depends on previously submitted patches
which have not been accepted, the new patch will not be accepted.
Please don't submit dependent patches until all prior prerequisite
patches are accepted.
Pedro:
diff --git a/gdb/regformats/microblaze-with-stack-protect.dat
...
Please send a preparatory, independent, patch that updates
features/Makefile instead and generates this file, in a new thread,
with self-contained description, following the
checklist:
https://sourceware.org/gdb/wiki/ContributionChecklist
Preparatory means that the patch should be submitted before the current patch.
I will be sending this patch soon.
OK. As mentioned above, please do not resubmit this patch until that
patch is submitted and accepted.
--
Michael Eager eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077