[PATCH] Add generic LE/BE load/store uint type convertors and use them in msgb

[Holger Hans Peter Freyther]

On Wed, Feb 05, 2014 at 09:09:37PM +0100, ☎ wrote:

> +/* Store unaligned n-byte integer (little-endian encoding) into uintXX_t */
> +static inline void osmo_storeXXle_ext(uintXX_t x, uint8_t *p, uint8_t n)
> +{
> +
> +
> +	uint8_t i;
> +	for(i = 0; i < n; p[i] = (x >> i * 8) & 0xFF, i++);
> +/*
> +
> +	uint8_t i, adj = 8 * (8 - n);
> +	uintXX_t y = (x << adj) >> adj;
> +	for(i = 0; i < n; p[i] = (y >> i * 8) & 0xFF, i++);
> +*/

left over comments or is that expanded in another way?

> +/* Store unaligned n-byte integer (big-endian encoding) into uintXX_t */
> +static inline void osmo_storeXXbe_ext(uintXX_t x, uint8_t *p, uint8_t n)
> +{
> +/*
> +	uint8_t i, adj = 8 * (8 - n);
> +	uintXX_t y = (x << adj) >> adj;
> +	for(i = 0; i < n; p[i] = (y >> ((n - 1 - i) * 8)) & 0xFF, i++);
> +*/

left over?


> +/*
> +   Less trivial LE/BE functions are autogenerated
> +   see included bitXXgen.h files
> +*/

What makes them more trivial?

> diff --git a/tests/bits/bitrev_test.c b/tests/bits/bitrev_test.c
> index 5eca990..6cf340e 100644
> --- a/tests/bits/bitrev_test.c
> +++ b/tests/bits/bitrev_test.c

> +char s[18], *p;

make static?

> +
> +void check_ls_64(uint8_t bytes)

static too

> +{
> +	uint8_t T[bytes], D = (8 - bytes), A = 8 * D, C = 2 * D;
> +	uint64_t _test = ((uint64_t)rand() << 32) + rand(), a_test = (_test << A) >> A;

that looks a bit crazy? What is your goal? Can you simplify it?


> +	if (0 != memcmp(s + C, p, bytes)) {
> +		printf("%s\t%s\t%u BE store FAILED!\n", s + C, p, bytes * 8);
> +	} else printf("%u BE store OK\n", bytes * 8);

coding style. :)

> +	if (0 != memcmp(s, p, 2)) {
> +		printf ("%s\t", s);
> +		printf ("%s\t", p);
> +		printf("16 BE FAILED on %" PRIx16 "\n");
> +	} else printf("16 BE store OK\n");

coding style. :)

> From ab98c898fa54e441fdb0b08145bad074c09f2cf1 Mon Sep 17 00:00:00 2001
> From: Max <max.suraev at fairwaves.ru>
> Date: Wed, 5 Feb 2014 20:10:38 +0100
> Subject: [PATCH 2/2] Add Kasumi cipher implementation
> +                       osmocom/core/bit32gen.h \
> +                       osmocom/core/bit64gen.h \

that is is in the wrong patch?

> +osmocom/core/bit%gen.h: osmocom/core/bitXXgen.h.tpl
> +	$(AM_V_GEN)$(MKDIR_P) $(dir $@)
> +	$(AM_V_GEN)sed -e's/XX/$*/g' $< > $@
> +

that is in the wrong patch too?

> +#ifndef __KASUMI_H__
> +#define __KASUMI_H__

_/__ are reserved for the system. I started to use "#pragma once"

> +uint64_t _kasumi(uint64_t P, uint16_t *KLi1, uint16_t *KLi2, uint16_t *KOi1, uint16_t *KOi2, uint16_t *KOi3, uint16_t *KIi1, uint16_t *KIi2, uint16_t *KIi3);

can you mark some of the parameters as const?

> +static uint16_t
> +_kasumi_FI(uint16_t I, uint16_t skey)

coding style. :)

> +	static uint16_t S7[] = {

Mark these arrays as const too. GCC is clever enough to figure out it is
static.

> +	static uint16_t S9[] = {

Same here.

> +	L  ^= (skey & 0x1FF);
> +	R ^= (skey >> 9);

Extra spaces?

> +
> +	L  = S9[L]  ^ R;
> +	R = S7[R] ^ (L & 0x7F);

Extra spaces?

> +static uint32_t
> +_kasumi_FO(uint32_t I, uint16_t *KOi1, uint16_t *KOi2, uint16_t *KOi3, uint16_t *KIi1, uint16_t *KIi2, uint16_t *KIi3, unsigned i)

coding style. :)

> +static uint32_t
> +_kasumi_FL(uint32_t I, uint16_t *KLi1, uint16_t *KLi2, unsigned i)

coding style. :)

> +uint64_t
> +_kasumi(uint64_t P, uint16_t *KLi1, uint16_t *KLi2, uint16_t *KOi1, uint16_t *KOi2, uint16_t *KOi3, uint16_t *KIi1, uint16_t *KIi2, uint16_t *KIi3)

coding style. :)

> +/*! \brief Expand key into set of subkeys
> + *  \param[in] key (128 bits) as array of bytes
> + *  \param[out] arrays of round-specific subkeys - see TS 135 202 for details
> + */
> +void
> +_kasumi_key_expand(const uint8_t *key, uint16_t *KLi1, uint16_t *KLi2, uint16_t *KOi1, uint16_t *KOi2, uint16_t *KOi3, uint16_t *KIi1, uint16_t *KIi2, uint16_t *KIi3)

commented again? make const?

> +	for (i = 0; i < 8; i++) /* Work with 16 bit subkeys and create prime subkeys */
> +	{
> +		C[i] ^= osmo_load16be(key + i * 2);
> +	}

coding style. :)

> +	/* C[] now stores K-prime[] */
> +	for (i = 0; i < 8; i++) /* Create round-specific subkeys */
> +	{

coding style. :)

> +void
> +_kasumi_kgcore(uint8_t CA, uint8_t cb, uint32_t cc, uint8_t cd, const uint8_t *ck, uint8_t *co, uint16_t cl)

coding style. :)

> +inline int _compare_mem(uint8_t * x, uint8_t * y, size_t len) {
> +    if (0 != memcmp(x, y, len)) {

static coding style (tabs vs. space)