Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
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



  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