Re: CRC review

From: Daryle Walker (darylew_at_[hidden])
Date: 2001-03-11 15:41:38

[Since I get the daily digest, I didn't see John's attachments.  This is
written before I looked for them.]

on 3/10/01 7:59 AM, John Maddock at John_Maddock_at_[hidden] wrote:

> I'm getting several problem with the crc review code, first the compile
> time problems:
> 
> crc.hpp doesn't build with either VC6 or C++ Builder, I've attatched a
> modified header file that fixes this, the changes are:
> 
> inconsistent use of class and struct (VC6)

Just noticed that inconsistency in "bit_traits," whoops.

> use of integral constant expressions not supported by C++ Builder.
> use of static constant data members changed to BOOST_STATIC_CONSTANT (for
> VC6).

I saw this new addition after I downloaded Boost 1.21.0.  I was wondering if
I should upload a new version of CRC with this change.  But I wouldn't have
made it before the review period began.  Should I reconsider?

> I also see the following compiler warnings from Borland:
> 
> [C++ Warning] CRC.HPP(92): W8055 Possible overflow in shift operation
> [C++ Warning] CRC.HPP(95): W8055 Possible overflow in shift operation
> 
> And these do look justified to me ??

You're talking about how I created the "sig_bits_" constants, right?  I'm
not sure what your warnings exactly mean, but I think that the bits are
supposed to be overflowing.  I created those constants differently in
previous versions of the package.  I changed it after Joel Young noticed
problems with the old method.

> At run time, VC6 asserts over the following:
> 
> // Simulate CRC-16
> crc_slow<16>  crc_16( 0x8005, 0, 0, true, true );
> crc_16( data, data + data_len );
> assert( *crc_16 == 0xBB3D );
> 
> and from Borland over:
> 
> // Simulate CRC-32
> crc_slow<32>  crc_32( 0x04C11DB7, 0xFFFFFFFF, 0xFFFFFFFF, true, true );
> crc_32( data, data_len );
> assert( *crc_32 == 0xCBF43926 );
> 
> Looks like the warnings were justified after all?

All of these tests worked out for me.  Have you checked to see what the
actual (wrong) numbers were?  Maybe the constants, on the right of the
equals operators, are written wrong.  I mean as the compiler picked an
incompatible type for them.  I originally wanted to use macros like UINT32_C
in <boost/cstdint.hpp>, but they're not #defined on my system(!), so I left
them bare.

A few weeks ago, Jens Maurer reported some minor compatibility problems with
unsigned types in GCC.  It seems that not even the basic unsigned operations
are that portable.  Ugh.

> That's all for now, more to come...

-- 
Daryle Walker
Mac, Internet, and Video Game Junkie
darylew AT mac DOT com