From: Kaushik Phatak <Kaushik.Phatak@kpitcummins.com>
To: Joel Brobecker <brobecker@adacore.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,
Yao Qi <yao@codesourcery.com>
Subject: RE: [RFA 3/5] New port: CR16: gdb port
Date: Tue, 09 Oct 2012 13:20:00 -0000 [thread overview]
Message-ID: <C6CA53A2A46BA7469348BDBD663AB65845B399F9@KCHJEXMB02.kpit.com> (raw)
In-Reply-To: <20121005122044.GS3028@adacore.com>
Hi Joel,
> I think that there is a log of stuff in cr16-tdep that is actually
> linux-specific, and that you should move to cr16-linux-tdep.c
I have moved register initializations into the cr16-linux-tdep.c. This
was also recommended by Yao.
> Take a look at how sparc is organized, or maybe amd64.
The other targets like amd64 have plenty on stuff in this file, however it is
unrelated to this port. I feel the updated cr16-linux-tdep.c file should allow
it to build on top of the bareboard one.
> Needs a comment. For global variables and constants
Added comments at appropriate location in the code.
> have an empty line between the documentation/comment and the function.
Added empty lines between comment and functions
>> +extern ins currInsn;
> This looks strange. Why extern, and where would they come from?
I have added comments for this and removed the unnecessary externs here.
> Don't these functions have declarations in opcode? This is really ugly
These prototypes were added recently and are part of a bfd patch. I forgot
to remove these in the last version.
> +extern const gdb_byte breakpoint_linux[];
I have removed this as it is not used in the later versions of the patch.
> The last line isn't aligned properly.
> So this paragraph needs to be reformatted.
> requires that 2 spaces be used after periods.
> Please also move the closing "*/" to the end of the last line
> The '{' needs to be on the next line.
> Trailing space at the end of the second line
> move the closing */ to the second line as well.
> And empty line between comment
> moved the last comment to just above the field
I have made another pass over the code and have taken care of the above
mentioned points and other formatting nits which I could find. Thanks.
I have not attached my updated patch here. I am working on Yao's comments
and will attach it in that reply.
Regards,
Kaushik
next prev parent reply other threads:[~2012-10-09 13:20 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-04 10:22 Kaushik Phatak
2012-10-04 14:48 ` Joel Brobecker
2012-10-05 11:44 ` Kaushik Phatak
2012-10-05 12:21 ` Joel Brobecker
2012-10-09 13:20 ` Kaushik Phatak [this message]
2012-10-08 6:59 ` Yao Qi
2012-10-09 15:03 ` Kaushik Phatak
2012-10-22 22:41 ` Joel Brobecker
2012-10-23 13:03 ` Kaushik Phatak
2012-10-23 13:55 ` Joel Brobecker
2012-10-26 5:15 ` Kaushik Phatak
2012-11-15 17:43 ` Joel Brobecker
2012-11-20 13:01 ` Kaushik Phatak
2012-11-22 17:50 ` Joel Brobecker
2013-01-08 10:02 ` Kaushik Phatak
2013-01-15 9:31 ` Kaushik Phatak
2013-01-17 8:59 ` Joel Brobecker
2013-01-18 7:41 ` Kaushik Phatak
2013-01-18 14:17 ` Joel Brobecker
2013-01-22 13:49 ` Kaushik Phatak
2013-01-22 15:43 ` Pedro Alves
2013-01-23 14:22 ` Kaushik Phatak
2013-01-23 14:26 ` Pedro Alves
2013-01-23 15:34 ` Pedro Alves
2013-06-19 13:30 ` Kaushik Phatak
2013-06-25 18:42 ` Pedro Alves
2013-06-26 7:08 ` Kaushik Phatak
2013-06-26 10:37 ` Pedro Alves
2013-01-18 18:25 ` Pedro Alves
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=C6CA53A2A46BA7469348BDBD663AB65845B399F9@KCHJEXMB02.kpit.com \
--to=kaushik.phatak@kpitcummins.com \
--cc=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=yao@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox