Re: [PATCH] tests/test-crc.c: New tests for future optimised implementation.

[Simon Josefsson]

Sam Russell <sam.h.russell@gmail.com> writes:

> +  char plaintext[] = "This file is free software: you can redistribu"
> +                     "te it and/or modify it under the terms of the "
> +                     "GNU Lesser General Public License as published"
> +                     " by th";

Using license texts like this has at least two problems: 1) license
texts aren't freely licensed so you can't re-use them (although this
snippet is probably too small to be copyrightable), and 2) this may
trigger license review checkers and could result in incorrect license
attribution for this file.

IMHO it isn't worth this hassle.  Use some other freely licensed random
text string instead.  Or repeat "Gnulib crc test string" many times.

> +  char data[128 + 16 + 16];

Use sizeof or some symbol instead of fixed sized like this.

> +   * Test for new CRC32 implementation
> +   * The original implementation works on a byte-by-byte basis
> +   * but the new one will work on 8 or 16 byte alignments, so
> +   * these tests will confirm correct operation with non-aligned
> +   * data and data that isn't even multiples of 16 in length.
> +   *
> +   * The PCLMUL implementation takes 128 bytes at a time on
> +   * 16-byte alignment, so we will do 128 + 16 bytes of plaintext
> +   * and alter the alignment up to 16 bytes

This isn't correct until the new implementation is added -- maybe you
could rewrite this into something simpler like "Test that the API handle
differently aligned data."?  I would make it test much larger
mis-alignment too, how about a ~700 byte string and test up to ~300 byte
mis-alignment?

/Simon

> +   */
> +
> +  for (i = 0; i < 16; i++)
> +    {
> +      memcpy(data + i, plaintext, 128 + 16);
> +      p = crc32_update_no_xor (0, data + i, 128 + 16);
> +      if (p != 0x18c9bfb0)
> +        {
> +          printf ("aligned c at %lu got %lx\n", i, (unsigned long) p);
> +          return 1;
> +        }
> +    }
> +
> +
>    return 0;
>  }
>

signature.asc
Description: PGP signature