Messages in this thread | | | Date | Sat, 29 Jan 2005 16:02:21 -0800 | From | Matt Mackall <> | Subject | Re: [PATCH 04/04] Add LRW |
| |
On Mon, Jan 24, 2005 at 12:57:50PM +0100, Fruhwirth Clemens wrote: > This is the core of my LRW patch. Added test vectors. > http://grouper.ieee.org/groups/1619/email/pdf00017.pdf
Please include a URL for the standard at the top of the LRW code and next to the test vectors. I had to search around a fair bit for decent background material, would be helpful to a couple other references as well.
> +static inline void findAlignment(u128 callersN, int value, int *align) { > + int i;
Your gfmulseq code has lots of StudlyCaps and strange whitespace, eg this '{' should be on the next line.
> + /* Copy N, so lsr does not destroy caller's copy */ > + u128_alloc(N); > + copy128(N,callersN);
The usage of your u128 type is really confusing, so 'u128' is an especially bad name. I expect u128 to work like u64 and u32. I propose gf128_t.
> + int i; // Outer control loop counter
C++ comments.
> +#define min(a,b) (a)<(b)?(a):(b)
Have a very nice one of those already.
> +#ifdef DEBUG > + printf("negative step at:"); > + print128(currentN); > +#endif
Better to use printk and put #define printk printf in your userspace test harness.
> +typedef u64 *u128; > +typedef u64 *u128_t;
Did I mention confusing?
> +#define u128_alloc(VAR) u64 _ ## VAR ## _[2]; u128 VAR = _ ## VAR ## _
Wrap this in a struct, please. That's disgusting.
> -obj-$(CONFIG_CRYPTO) += api.o scatterwalk.o cipher.o digest.o compress.o \ > +obj-$(CONFIG_CRYPTO) += api.o scatterwalk.o cipher.o digest.o compress.o lrw.o gfmulseq.o \
LRW and the GF(2**128) code is not configurable?
-- Mathematics is the supreme nostalgia of our time. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |