Re: [PATCH] Read arm core files

[Richard Earnshaw <rearnsha at arm dot com>]

On Wed, 2006-02-15 at 11:04, Girish Shilamkar wrote:
> Hi,
> 	The patch enables gdb to read arm core files.
> -Girish.

Girish,

This needs some work yet before it can be accepted.

BTW, do you have a copyright assignment in place?  If not, then we will
need one before we can proceed any further.

Finally, please make sure your patches are against the most recent
version of CVS rather than a release.  We don't incorporate new code
onto existing releases.

R.

 2006-02-15   Girish Shilamkar  <girish@linsyssoft.com>
 
        * arm-linux-tdep.c Core file handler has been added. 
        * config/arm/linux.mt Compiles corelow.c required for 
        deprecated_add_core_fns 

Please take a closer look at existing ChangeLog entries and try to copy
the style there.   All entries are of the form
	* <file modified>: (<function modified>) Sentence
	describing change, ending with full stop.


+/* Following enums used to implement the core file support. */

Two spaces at the end of a sentence, even if it ends the entire
comment.  Your sentence isn't grammatically correct either:  "The
following enums are used to implement core file support."

+enum {
+  ELF_NGREG = 18,      /* core reg size is 72 */
All comments start with a capital letter.

 void
+arm_linux_supply_gregset (char *buf)

All functions must have a comment that precedes them describing what
they do -- even in cases like this where it is fairly obvious.


+     regcache_raw_supply (current_regcache, FP0_REGNUM + regi, buf + 8 * regi);

Watch out for line length.  Wrap your code *before* you reach column 80.


+        warning ("wrong size gregset struct in core file");

You need to take care of i18n here.  Wrap your strings thus _("My string
for translation").