Samuel Pitoiset <[email protected]> writes:
> +static const uint32_t orig_p[BF_ROUNDS + 2] = {
> + 0x243F6A88u, 0x85A308D3u, 0x13198A2Eu, 0x03707344u,
The u suffix is not needed here. Integer constants have whatever type
is necessary to represent the value, and implicit conversion of signed
to unsigned is perfectly fine.
> +static void F(AVBlowfish *bf, uint32_t *xl, uint32_t *xr, int i)
> +{
> + uint32_t y;
> +
> + *xl ^= bf->p[i];
> + y = bf->s[0][(*xl >> 24) & 0xFF];
> + y += bf->s[1][(*xl >> 16) & 0xFF];
> + y ^= bf->s[2][(*xl >> 8) & 0xFF];
> + y += bf->s[3][ *xl & 0xFF];
> + *xr ^= y;
> +
> + FFSWAP(uint32_t, *xl, *xr);
Loading the *xl and *xr values into local variables would look nicer and
might give better code if the compiler doesn't realise that xl and xr
are always unequal.
> +}
> +
> +static void blowfish_crypt(AVBlowfish *bf, uint32_t *xl, uint32_t *xr,
> + int direction)
> +{
> + uint32_t Xl, Xr;
> + int i;
> +
> + Xl = *xl;
> + Xr = *xr;
> +
> + if (direction) {
> + for (i = 0; i < BF_ROUNDS; ++i)
> + F(bf, &Xl, &Xr, i);
> + } else {
> + for (i = BF_ROUNDS + 1; i > 1; --i)
> + F(bf, &Xl, &Xr, i);
> + }
> +
> + FFSWAP(uint32_t, Xl, Xr);
This FFSWAP is pointless. You can simply swap the names in the code below.
> + if (direction) {
> + Xr = Xr ^ bf->p[BF_ROUNDS];
> + Xl = Xl ^ bf->p[BF_ROUNDS + 1];
> + } else {
> + Xr = Xr ^ bf->p[1];
> + Xl = Xl ^ bf->p[0];
> + }
> +
> + *xl = Xl;
> + *xr = Xr;
> +}
There is practically no code in common between the two directions. It
would probably be clearer to simply put the code for each case directly
in the functions below.
> +void av_blowfish_init(AVBlowfish *bf, const uint8_t *key, int key_len)
> +{
> + uint32_t data, datal, datar;
> + int i, j, k;
> +
> + memcpy(bf->s, orig_s, sizeof(orig_s));
> +
> + j = 0;
> + for (i = 0; i < BF_ROUNDS + 2; ++i) {
> + data = 0x00000000;
> + for (k = 0; k < 4; k++) {
> + data = (data << 8) | key[j++];
> + if (j >= key_len)
> + j = 0;
> + }
> + bf->p[i] = orig_p[i] ^ data;
> + }
> +
> + datal = 0x00000000;
> + datar = 0x00000000;
What I said before about using plain 0.
> + for (i = 0; i < BF_ROUNDS + 2; i += 2) {
> + av_blowfish_encrypt(bf, &datal, &datar);
> + bf->p[i] = datal;
> + bf->p[i + 1] = datar;
> + }
> +
> + for (i = 0; i < 4; ++i) {
> + for (j = 0; j < 256; j += 2) {
> + av_blowfish_encrypt(bf, &datal, &datar);
> + bf->s[i][j] = datal;
> + bf->s[i][j + 1] = datar;
> + }
> + }
> +}
> +
> +void av_blowfish_encrypt(AVBlowfish *bf, uint32_t *xl, uint32_t *xr)
> +{
> + blowfish_crypt(bf, xl, xr, 1);
> +}
> +
> +void av_blowfish_decrypt(AVBlowfish *bf, uint32_t *xl, uint32_t *xr)
> +{
> + blowfish_crypt(bf, xl, xr, 0);
> +}
> +
> +#ifdef TEST
> +#include <stdio.h>
> +#undef printf
> +
> +#define NUM_VARIABLE_KEY_TESTS 34
> +
> +/* plaintext bytes -- left halves */
> +uint32_t plaintext_l[NUM_VARIABLE_KEY_TESTS] = {
> + 0x00000000l, 0xFFFFFFFFl, 0x10000000l, 0x11111111l, 0x11111111l,
> + 0x01234567l, 0x00000000l, 0x01234567l, 0x01A1D6D0l, 0x5CD54CA8l,
> + 0x0248D438l, 0x51454B58l, 0x42FD4430l, 0x059B5E08l, 0x0756D8E0l,
> + 0x762514B8l, 0x3BDD1190l, 0x26955F68l, 0x164D5E40l, 0x6B056E18l,
> + 0x004BD6EFl, 0x480D3900l, 0x437540C8l, 0x072D43A0l, 0x02FE5577l,
> + 0x1D9D5C50l, 0x30553228l, 0x01234567l, 0x01234567l, 0x01234567l,
> + 0xFFFFFFFFl, 0x00000000l, 0x00000000l, 0xFFFFFFFFl };
Drop the l suffixes.
[...]
> +/**
> + * @brief Encrypts using the Blowfish algorithm.
> + *
> + * @param bf an AVBlowfish context
> + * @param xl left eight bytes halves of input to be encrypted
> + * @param xr right eight bytes halves of input to be encrypted
> + */
> +void av_blowfish_encrypt(struct AVBlowfish *bf, uint32_t *xl, uint32_t *xr);
Eight bytes where? uint32_t is four bytes. Whatever this is trying to
say, it is confusing.
--
Måns Rullgård
[email protected]
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
