Re: [ECOS] Re: Bug in crc32 routine?

[Jonathan Larmour <jifl at eCosCentric dot com>]

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 :).

> --- 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.

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 ;-)).

> +++ 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.

> Index: packages/services/crc/current/include/crc.h
> ===================================================================
> RCS file: /cvs/ecos/ecos/packages/services/crc/current/include/crc.h,v
> retrieving revision 1.1
> diff -u -r1.1 crc.h
> --- packages/services/crc/current/include/crc.h	9 Aug 2002 10:27:04 -0000	1.1
> +++ packages/services/crc/current/include/crc.h	11 Oct 2002 11:32:33 -0000
> @@ -65,6 +65,23 @@
>  unsigned long 
>  cyg_crc32(unsigned char *s, int len);
>  
> +// Gary S. Brown's 32 bit CRC, but accumulate the result from a
> +// previous CRC calculation
> +
> +unsigned long 
> +cyg_crc32_accumulate(unsigned long crc, unsigned char *s, int len);
[snip rest]

These should have C++ safe extern definitions. I've added these (change 
attached).

> +  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).

Jifl
--
eCosCentric       http://www.eCosCentric.com/       <info@eCosCentric.com>
--[ "You can complain because roses have thorns, or you ]--
--[  can rejoice because thorns have roses." -Lincoln   ]-- Opinions==mine
Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/services/crc/current/ChangeLog,v
retrieving revision 1.2
diff -u -5 -p -r1.2 ChangeLog
--- ChangeLog	14 Oct 2002 15:31:10 -0000	1.2
+++ ChangeLog	18 Oct 2002 02:10:08 -0000
@@ -1,5 +1,9 @@
+2002-10-18  Jonathan Larmour  <jifl@eCosCentric.com>
+
+	* include/crc.h: Add extern definitions.
+
 2002-10-11  Andrew Lunn  <andrew.lunn@ascom.ch>
 
 	* src/crc32.c (cyg_ether_crc32):
 	* src/crc32.c (cyg_ether_crc32_accumulate): New functions for Ethernet
 	  FCS style CRC calculations.
Index: include/crc.h
===================================================================
RCS file: /cvs/ecos/ecos/packages/services/crc/current/include/crc.h,v
retrieving revision 1.2
diff -u -5 -p -r1.2 crc.h
--- include/crc.h	14 Oct 2002 15:31:13 -0000	1.2
+++ include/crc.h	18 Oct 2002 02:10:08 -0000
@@ -53,40 +53,48 @@
 //==========================================================================
 
 #ifndef _SERVICES_CRC_CRC_H_
 #define _SERVICES_CRC_CRC_H_
 
+#ifndef __externC
+# ifdef __cplusplus
+#  define __externC extern "C"
+# else
+#  define __externC extern
+# endif
+#endif
+
 // Compute a CRC, using the POSIX 1003 definition
 
-unsigned long 
+__externC unsigned long 
 cyg_posix_crc32(unsigned char *s, int len);
 
 // Gary S. Brown's 32 bit CRC
 
-unsigned long 
+__externC unsigned long 
 cyg_crc32(unsigned char *s, int len);
 
 // Gary S. Brown's 32 bit CRC, but accumulate the result from a
 // previous CRC calculation
 
-unsigned long 
+__externC unsigned long 
 cyg_crc32_accumulate(unsigned long crc, unsigned char *s, int len);
 
 // Ethernet FCS Algorithm
 
-unsigned long 
+__externC unsigned long 
 cyg_ether_crc32(unsigned char *s, int len);
 
 // Ethernet FCS algorithm, but accumulate the result from a previous
 // CRC calculation.
 
-unsigned long 
+__externC unsigned long 
 cyg_ether_crc32_accumulate(unsigned long crc, unsigned char *s, int len);
 
 // 16 bit CRC with polynomial x^16+x^12+x^5+1
 
-unsigned short
+__externC unsigned short
 cyg_crc16(unsigned char *s, int len);
 
 #endif // _SERVICES_CRC_CRC_H_