This is the mail archive of the
binutils@sourceware.cygnus.com
mailing list for the binutils project.
Re: Binutils Bug on i960 file format
- To: careri at public dot sta dot net dot cn
- Subject: Re: Binutils Bug on i960 file format
- From: Nick Clifton <nickc at cygnus dot com>
- Date: Mon, 22 May 2000 10:55:48 -0700
- CC: binutils at sourceware dot cygnus dot com
Hi Su,
Thanks very much for submitting this patch.
: Changelog entry
: *cpu-i960.c(scan_960_mach) :add some code to recongnize the i960 machine architecture describing as "80960KB".
I have a couple of comments on patch submission in general that helps
me, and other readers of this list, understand patches that are submitted:
* Please format your ChangeLog entry(s) in the same way that other
Changelog entries are formatted. In particular the entry that you
supplied did not include your name and email address, was not
wrapped to 80 columns, and was not written as a normal,
descriptive English sentance. I would suggest reformating your
entry like this:
2000-05-22 Su Wenming <careri@public.sta.net.cn>
* cpu-i960.c (scan_960_mach): Add code to recongnize the i960
machine architecture described as "80960KB".
* If possible, please include a patch as plain ASCII, rather than as
encoed attachment. This helps old fogies like me who do not have
an attachment enabled email reader. (I have to save your email to
a temporary, switch to a shell, run metamail on the file, save the
decoded patch, then switch back to my editor and load up the
decoded file, all just to be able to examine the patch. If it was
in plain text, along with the rest of your email, it would be much
easier for me to read). Of course if the patch is very long, then
it makes sense to compress and encode it. But for short patches,
plain text is preferable.
* When generating a patch please use 'diff -p' to generate a context
patch, and please try to have the unpatched file as the "old" or
first file and the patched file as the "new" or second file. The
patch you generated had the two files swapped, so it looked like
you were actually removing your new code from the cpu-i960.c file:
37,41d36
< int i;
<
<
< for( i = 0; i < strlen(string); i ++ )
< string[i] = tolower( string[i] );
44c39
< /*in some bfd the cpu-id is written as "80960KB"*/
---
>
* In order for me to be able to accept a patch it is necessary for
you to have a copyright assignment on file with the FSF. It helps
me if you can mention whether you do have such an assignment on
file, when you submit a patch.
Finally, on a technical note, your patch was not quite complete, in
that it would recognise some illegal names, such as "80960core". I
propose the following alternative patch which I belive should do the
right thing, and which also takes the opportunity to tidy up the code
formating in the scan_960_match() function. Please let me know if you
find this version acceptable.
Cheers
Nick
Index: bfd/cpu-i960.c
===================================================================
RCS file: /cvs/src//src/bfd/cpu-i960.c,v
retrieving revision 1.1.1.1
diff -p -r1.1.1.1 cpu-i960.c
*** cpu-i960.c 1999/05/03 07:28:55 1.1.1.1
--- cpu-i960.c 2000/05/22 17:51:21
*************** scan_960_mach (ap, string)
*** 34,61 ****
const char *string;
{
unsigned long machine;
! /* Look for the string i960, or somesuch at the front of the string */
! if (strncmp("i960",string,4) == 0) {
! string+=4;
! }
! else {
! /* no match, can be us */
! return false;
! }
! if (string[0] == 0) {
! /* i960 on it's own means core to us*/
! if (ap->mach == bfd_mach_i960_core) return true;
! return false;
! }
! if (string[0] != ':') {
return false;
! }
! string++;
if (string[0] == '\0')
return false;
if (string[0] == 'c' && string[1] == 'o' && string[2] == 'r' &&
string[3] == 'e' && string[4] == '\0')
machine = bfd_mach_i960_core;
--- 34,69 ----
const char *string;
{
unsigned long machine;
+ int i;
! for (i = 0; i < strlen (string); i ++)
! string[i] = tolower (string[i]);
! /* Look for the string i960 at the front of the string. */
! if (strncmp ("i960", string, 4) == 0)
! {
! string += 4;
! /* i960 on it's own means core to us. */
! if (string[0] == 0)
! return ap->mach == bfd_mach_i960_core;
!
! /* "i960:*" is valid, anything else is not. */
! if (string[0] != ':')
! return false;
!
! string ++;
! }
! /* In some bfds the cpu-id is written as "80960KB". */
! else if (strcmp ("80960kb", string) == 0)
! return ap->mach == bfd_mach_i960_kb_sb;
! /* No match, can't be us. */
! else
return false;
!
if (string[0] == '\0')
return false;
+
if (string[0] == 'c' && string[1] == 'o' && string[2] == 'r' &&
string[3] == 'e' && string[4] == '\0')
machine = bfd_mach_i960_core;
*************** scan_960_mach (ap, string)
*** 63,69 ****
machine = bfd_mach_i960_ka_sa;
else if (strcmp (string, "kb_sb") == 0)
machine = bfd_mach_i960_kb_sb;
! else if (string[1] == '\0' || string[2] != '\0') /* rest are 2-char */
return false;
else if (string[0] == 'k' && string[1] == 'b')
machine = bfd_mach_i960_kb_sb;
--- 71,77 ----
machine = bfd_mach_i960_ka_sa;
else if (strcmp (string, "kb_sb") == 0)
machine = bfd_mach_i960_kb_sb;
! else if (string[1] == '\0' || string[2] != '\0') /* rest are 2-char. */
return false;
else if (string[0] == 'k' && string[1] == 'b')
machine = bfd_mach_i960_kb_sb;
*************** scan_960_mach (ap, string)
*** 85,91 ****
machine = bfd_mach_i960_hx;
else
return false;
! if (machine == ap->mach) return true;
return false;
}
--- 93,102 ----
machine = bfd_mach_i960_hx;
else
return false;
!
! if (machine == ap->mach)
! return true;
!
return false;
}