This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
Re: [ECOS] Re: Bug in crc32 routine?
On Fri, Oct 18, 2002 at 03:14:24AM +0100, Jonathan Larmour wrote:
> Andrew Lunn wrote:
> >I checked around and found a few more CRC calculations. In summary
> >
> >1) zlib
> >2) jffs2
> >3) gdb stubs
> >4) cygmon
> >5) ks32c5000 net driver
> >6) Redboot - already using cyg_crc32()
> [snip]
>
> Belated reply from me (what's new!), but just a few comments:
>
> First of all, we aren't the master for jffs2. If there's a problem with
> the crc32 calc for jffs2 we should tell <dwmw2@redhat.com>. But then I'm
> not convinced a mismatch with zlib counts as a problem. If you agree, then
> this is a nop :).
I say its a NOP. There is nothing wrong with using a different
algorithm. It makes little difference to code size since they (now)
shares the same table of coefficients, which is the dominant factor.
>
> >--- packages/devs/eth/arm/ks32c5000/current/cdl/ks32c5000_eth.cdl 23
> >May 2002 23:00:40 -0000 1.2
> >+++ packages/devs/eth/arm/ks32c5000/current/cdl/ks32c5000_eth.cdl 11
> >Oct 2002 11:32:31 -0000
> >@@ -58,6 +58,8 @@
> > active_if CYGPKG_IO_ETH_DRIVERS
> > implements CYGHWR_NET_DRIVERS
> > implements CYGHWR_NET_DRIVER_ETH0
> >+ requires (CYGPKG_CRC || (!(CYG_HAL_CPUTYPE == \"KS32C5000A\" )))
> >+
>
> More out of interest than anything, CDL has an implies operator which
> allows you to use:
>
> requires { (CYG_HAL_CPUTYPE == \"KS32C5000A\" ) implies CYGPKG_CRC }
>
> which is more readable.
Yep, forgot about that.
Although, in the past i've had problems with implies. I think implies
implies the CDL conflict engine can modify these options to try to
resolve the conflict. As you point out below, CYCPKG_CRC may not be
know, so it may actually try to resolve this conflict by changing the
CPUTYPE! This is all theoretical, i've not tried it.
> One thing I'm concerned about though is that the "default" requirements of
> the eth driver include a package that is not included by default. It shows
> up two long-standing CDL issues:
>
> - only hardware packages can be listed in ecos.db targets
> - there is no way for the user to request automatically loading required
> packages not currently in the config
>
> the former would be easy to fix, but would be a band-aid for the latter.
> Bart's on holiday for a week, but I'd be interested in his feedback. A
> bugzilla bug should probably be submitted to track this (depending on what
> the correct value of "this" is ;-)).
This is a deficiency i noted when testing. I was in two minds if to
add the CRC package to the net template, but decided not to since its
only required by one driver.
I'll leave bugzilla to you.
> >+++ packages/services/crc/current/doc/crc.sgml 11 Oct 2002 11:32:33
> >-0000
> >@@ -5,7 +5,7 @@
> > The CRC package provides implementation of CRC algorithms. This
> > includes the POSIX CRC calculation which produces the same result as
> > the cksum command on Linux, another 32 bit CRC by Gary S. Brown and a
> >-16bit CRC.
> >+16bit CRC. The CRC used for Ethernet FCS is also implemented.
> > </PARA>
> > </PARTINTRO>
> > <CHAPTER id="crc-functions">
> >@@ -35,17 +35,45 @@
> > <sect2 id="services-crc-api-cyg-crc32">
> > <title>cyg_crc32</title>
> > <para>
> >-This function implements a 32 bit CRC by Gary S. Brown. It uses the
> >+These functions implements a 32 bit CRC by Gary S. Brown. It uses the
>
> implement :). I've just gone ahead and tweaked this in the repo, and a few
> other trivial speeling typos.
No problem. I explicitly said i cannot spell! Well, its more likely
the words are spelled correctly, its just the wrong word!
> >+ if (1667500021 != cyg_ether_crc32(license_txt,sizeof(license_txt)-1)) {
>
> Mumble, mumble, 32-bit reliance, mumble ;-). Really, that's what the type
> defs in <cyg/infra/cyg_type.h> are for - rather than using unsigned longs.
> (Although in due course we should use the C99 <inttypes.h> when GCC gets
> round to implementing it).
OK, maybe, if i can find some free time with nothing more exciting to
do...
Andrew