Re: [PATCH 1/2] xfs: add CRC infrastructure

Dave Chinner
2012-11-07T20:29:11Z

On Wed, Nov 07, 2012 at 08:21:27AM -0800, Andi Kleen wrote:
> Dave Chinner <david@fromorbit.com> writes:
> > + */
> > +static inline __uint32_t
> > +xfs_start_cksum(char *buffer, size_t length, unsigned long cksum_offset)
> > +{
> > + __uint32_t zero = 0;
> > + __uint32_t crc;
> > +
> > + /* Calculate CRC up to the checksum. */
> > + crc = crc32c(XFS_CRC_SEED, buffer, cksum_offset);
> > +
> > + /* Skip checksum field */
> > + crc = crc32c(crc, &zero, sizeof(__u32));
> > +
> > + /* Calculate the rest of the CRC. */
> > + return crc32c(crc, &buffer[cksum_offset + sizeof(__be32)],
> > + length - (cksum_offset + sizeof(__be32)));
>
> Don't you need to remap a zero result to some other value, otherwise a
> real zero checksum will never be checked?

Why would that be necessary? We never include the checksum field in
the calculation when setting it or verifiying it, and the
verification uses exactly the same method as the original
calculation to check the CRC, so it doesn't matter if the CRC value
is zero or not - if it matches (zero or otherwise), the validation
passes....

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com