> Or repeat "Gnulib crc test string" many times. done
> This isn't correct until the new implementation is added
done
> I would make it test much larger
> mis-alignment too, how about a ~700 byte string and test up to ~300 byte
> mis-alignment?
I'm not sure what problem this solves. I've used a >128 byte test string
because I have the PCLMUL implementation in mind, but ultimately we're
testing mis-alignment on an 8-byte word (slice-by-8 reading into a 64-bit
register) or a 16-byte word (PCLMUL reading into a 128-bit register), so we
could get away with a single off-by-1 test but here we are testing
everything from 0-15 bytes of misalignment; a 49-byte misalignment should
create the same problem as a 1-byte misalignment.
On Tue, 15 Oct 2024 at 09:57, Simon Josefsson <si...@josefsson.org> wrote:
> Sam Russell <sam.h.russ...@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;
> > }
> >
>
gnulib_new_tests.patch
Description: Binary data
