This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
Re: redboot patch to make it compiling with gcc4 and higher
- From: Gary Thomas <gary at mlbassoc dot com>
- To: Andrea Michelotti <amichelotti at atmel dot com>
- Cc: Andrew Lunn <andrew dot lunn at ascom dot ch>,eCos patches <ecos-patches at ecos dot sourceware dot org>
- Date: Wed, 01 Dec 2004 07:46:51 -0700
- Subject: Re: redboot patch to make it compiling with gcc4 and higher
- Organization: MLB Associates
- References: <0abb01c4d78f$69c3eec0$0b0110ac@ipitec.it>
On Wed, 2004-12-01 at 03:20, Andrea Michelotti wrote:
> Hi Andrew,
> I compiled eCos using gcc4 (under cvs), It seems working fine.
> I had problem only with redboot that has some cast-as-lvalue and
> this is not supported in gcc4 any more
> (http://gnu.signal42.com/software/gcc/gcc-4.0/changes.html)
> I did this this little patch to make redboot compiling.
Thanks for looking into this. I have some comments and I think
there are some errors as well.
>> Index: src/mfill.c
>> ===================================================================
>> RCS file: /cvs/ecos/ecos/packages/redboot/current/src/mfill.c,v
>> retrieving revision 1.2
>> diff -u -5 -p -r1.2 mfill.c
>> --- src/mfill.c 24 Feb 2004 14:15:15 -0000 1.2
>> +++ src/mfill.c 1 Dec 2004 09:54:48 -0000
>> @@ -95,19 +95,20 @@ do_mfill(int argc, char *argv[])
>> }
>> // No checks here
>> if (set_8bit) {
>> // Fill 8 bits at a time
>> while ((len -= sizeof(cyg_uint8)) >= 0) {
>> - *((cyg_uint8 *)base)++ = (cyg_uint8)pat;
>> + *(cyg_uint8 *)base = (cyg_uint8)pat,base+=sizeof(cyg_uint8);
>> }
>> } else if (set_16bit) {
>> // Fill 16 bits at a time
>> while ((len -= sizeof(cyg_uint16)) >= 0) {
>> - *((cyg_uint16 *)base)++ = (cyg_uint16)pat;
>> + *(cyg_uint16 *)base = (cyg_uint16)pat,base+=sizeof(cyg_uint16);
>> }
>> } else {
>> // Default - 32 bits
>> while ((len -= sizeof(cyg_uint32)) >= 0) {
>> - *((cyg_uint32 *)base)++ = (cyg_uint32)pat;
>> + *(cyg_uint32*)base= pat,base+=sizeof(cyg_uint32);
>> +
>> }
>> }
>> }
These changes would [possibly] prevent a compiler from using
auto-increment addressing. A much better solution would be
something like this:
if (set_8bit) {
cyg_uint8 *bp = (cyg_uint8 *)base;
// Fill 8 bits at a time
while ((len -= sizeof(cyg_uint8)) >= 0) {
*bp++ = (cyg_uint8)pat;
}
Similarly for the other sizes.
>> Index: src/mcmp.c
>> ===================================================================
>> RCS file: /cvs/ecos/ecos/packages/redboot/current/src/mcmp.c,v
>> retrieving revision 1.2
>> diff -u -5 -p -r1.2 mcmp.c
>> --- src/mcmp.c 24 Feb 2004 14:15:15 -0000 1.2
>> +++ src/mcmp.c 1 Dec 2004 09:54:49 -0000
>> @@ -92,40 +92,50 @@ do_mcmp(int argc, char *argv[])
>> }
>> // No checks here
>> if (set_8bit) {
>> // Compare 8 bits at a time
>> while ((len -= sizeof(cyg_uint8)) >= 0) {
>> - if (*((cyg_uint8 *)src_base)++ != *((cyg_uint8 *)dst_base)++) {
>> - ((cyg_uint8 *)src_base)--;
>> - ((cyg_uint8 *)dst_base)--;
>> + if (*((cyg_uint8 *)src_base) != *((cyg_uint8 *)dst_base)) {
>> + src_base-=sizeof(cyg_uint8);
>> + dst_base-=sizeof(cyg_uint8);
>> +
>> diag_printf("Buffers don't match - %p=0x%02x, %p=0x%02x\n",
>> src_base, *((cyg_uint8 *)src_base),
>> dst_base, *((cyg_uint8 *)dst_base));
>> return;
>> }
>> + src_base+=sizeof(cyg_uint8);
>> + dst_base+=sizeof(cyg_uint8);
>> }
>> } else if (set_16bit) {
>> // Compare 16 bits at a time
>> while ((len -= sizeof(cyg_uint16)) >= 0) {
>> - if (*((cyg_uint16 *)src_base)++ != *((cyg_uint16 *)dst_base)++) {
>> - ((cyg_uint16 *)src_base)--;
>> - ((cyg_uint16 *)dst_base)--;
>> - diag_printf("Buffers don't match - %p=0x%04x, %p=0x%04x\n",
>> + if (*((cyg_uint16 *)src_base) != *((cyg_uint16 *)dst_base)) {
>> + src_base-=sizeof(cyg_uint16);
>> + dst_base-=sizeof(cyg_uint16);
>> + diag_printf("Buffers don't match - %p=0x%04x, %p=0x%04x\n",
>> src_base, *((cyg_uint16 *)src_base),
>> dst_base, *((cyg_uint16 *)dst_base));
>> return;
>> }
>> + src_base+=sizeof(cyg_uint16);
>> + dst_base+=sizeof(cyg_uint16);
>> +
>> }
>> } else {
>> // Default - 32 bits
>> while ((len -= sizeof(cyg_uint32)) >= 0) {
>> - if (*((cyg_uint32 *)src_base)++ != *((cyg_uint32 *)dst_base)++) {
>> - ((cyg_uint32 *)src_base)--;
>> - ((cyg_uint32 *)dst_base)--;
>> + if (*((cyg_uint32 *)src_base) != *((cyg_uint32 *)dst_base)) {
>> + src_base-=sizeof(cyg_uint32);
>> + dst_base-=sizeof(cyg_uint32);
>> +
>> diag_printf("Buffers don't match - %p=0x%08x, %p=0x%08x\n",
>> src_base, *((cyg_uint32 *)src_base),
>> dst_base, *((cyg_uint32 *)dst_base));
>> return;
>> }
>> + src_base+=sizeof(cyg_uint32);
>> + dst_base+=sizeof(cyg_uint32);
>> +
>> }
>> }
>> }
>>
The same comments apply here. Also note that your code is
incorrect for the if() cases where there is a mis-match, as
the pointers will be off by one.
--
Gary Thomas <gary@mlbassoc.com>
MLB Associates