Re: [Patch] Microblaze: Port of Linux gdbserver

[Michael Eager <eager@eagerm.com>]

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