On Thu, Jun 28, 2012 at 12:13:23PM +0200, Samuel Pitoiset wrote:
> ---
>  configure                |    4 +
>  doc/protocols.texi       |    9 +
>  libavformat/Makefile     |    2 +
>  libavformat/allformats.c |    2 +
>  libavformat/rtmpdh.c     |  228 ++++++++++++++++++++
>  libavformat/rtmpdh.h     |  106 +++++++++
>  libavformat/rtmpenc.c    |  532 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  libavformat/rtmpenc.h    |   58 +++++
>  libavformat/rtmpproto.c  |   80 +++++++-
>  9 files changed, 1016 insertions(+), 5 deletions(-)
>  create mode 100644 libavformat/rtmpdh.c
>  create mode 100644 libavformat/rtmpdh.h
>  create mode 100644 libavformat/rtmpenc.c
>  create mode 100644 libavformat/rtmpenc.h

This is missing a changelog entry, a version bump and an entry in
doc/general.texi.  Please consult the new formats checklist in the
developer documentation (again).

Get rid of the stray tabs that snuck into your code.

Does this pass "make check"?

> --- /dev/null
> +++ b/libavformat/rtmpdh.c
> @@ -0,0 +1,228 @@
> +
> +#include "avformat.h"
> +
> +#include "libavutil/sha.h"
> +
> +#include "internal.h"
> +#include "rtmp.h"
> +#include "rtmpdh.h"
> +#include "rtmpenc.h"
> +#include "url.h"

Separating the avformat.h #include looks weird, whatever...

> +/* RFC 2631, Section 2.1.5, http://www.ietf.org/rfc/rfc2631.txt */
> +static int dh_is_valid_public_key(ff_bignum y, ff_bignum p, ff_bignum q)
> +{
> +    ff_bignum bn = NULL;
> +    int ret = 0;
> +
> +    ff_bn_new(bn);
> +    if (!bn) {
> +        ret = AVERROR(ENOMEM);
> +        goto fail;
> +    }

Just return AVERROR(ENOMEM) directly here.  I don't think there is a
point in freeing memory that was never allocated.

> +    if (ff_bn_cmp_1(bn) != 0) {

Drop the " != 0".

> +ff_dh *ff_dh_init(int key_len)
> +{
> +    ff_dh *dh;
> +    int ret;
> +
> +    if (!(dh = ff_dh_new())) {
> +        ret = AVERROR(ENOMEM);
> +        goto fail;
> +    }

see above

> +    /* convert the public key value into big-endian form */
> +    memset(pub_key, 0, pub_key_len);
> +    ff_bn_bn2bin(dh->pub_key, pub_key + (pub_key_len - len), len);

pointless ()

> +    /* convert the big-endian form of the public key into a bignum */
> +    ff_bn_bin2bn(pub_key_bn, pub_key, pub_key_len);
> +    if (!pub_key_bn) {
> +        ret = AVERROR(ENOMEM);
> +        goto fail;
> +    }

see above

> +    /* when the public key is valid we have to compute the shared secret key 
> */
> +    if ((ret = dh_is_valid_public_key(pub_key_bn, dh->p, q1)) < 0) {
> +        goto fail;
> +    } else {
> +        if ((ret = ff_dh_compute_key(secret, pub_key_len, pub_key_bn, dh)) < 
> 0) {
> +            ret = AVERROR(EINVAL);
> +            goto fail;
> +        }
> +    }

Merge the else and the if.

> --- /dev/null
> +++ b/libavformat/rtmpdh.h
> @@ -0,0 +1,106 @@
> +
> +#ifndef AVFORMAT_RTMPDH_H
> +#define AVFORMAT_RTMPDH_H
> +
> +#include "avformat.h"
> +
> +#if CONFIG_OPENSSL
> +#include <openssl/bn.h>
> +#include <openssl/dh.h>

You are missing a config.h #include for this to work properly.

> +/* http://www.ietf.org/rfc/rfc3526.txt */
> +/* 2^1024 - 2^960 - 1 + 2^64 * { [2^894 pi] + 129093 } */
> +#define P1024 \
> +    "FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD1" \
> +    "29024E088A67CC74020BBEA63B139B22514A08798E3404DD" \
> +    "EF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245" \
> +    "E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7ED" \
> +    "EE386BFB5A899FA5AE9F24117C4B1FE649286651ECE65381" \
> +    "FFFFFFFFFFFFFFFF"
> +
> +/* Group morder largest prime factor: */
> +#define Q1024 \
> +    "7FFFFFFFFFFFFFFFE487ED5110B4611A62633145C06E0E68" \
> +    "948127044533E63A0105DF531D89CD9128A5043CC71A026E" \
> +    "F7CA8CD9E69D218D98158536F92F8A1BA7F09AB6B6A8E122" \
> +    "F242DABB312F3F637A262174D31BF6B585FFAE5B7A035BF6" \
> +    "F71C35FDAD44CFD2D74F9208BE258FF324943328F67329C0" \
> +    "FFFFFFFFFFFFFFFF"

Align the \.

> --- /dev/null
> +++ b/libavformat/rtmpenc.c
> @@ -0,0 +1,532 @@
> +
> +#include "internal.h"
> +#include "rtmp.h"
> +#include "rtmpdh.h"
> +#include "rtmpenc.h"
> +#include "url.h"
> +
> +#include "libavutil/rc4.h"

Move the libavutil #include before the others.

> +static unsigned int get_dh_offset(const uint8_t *buf)
> +{
> +    int i, dh_offset = 0;
> +
> +    for (i = 768; i < 772; i++)
> +        dh_offset += buf[i];
> +    dh_offset = (dh_offset % 632) + 8;

pointless ()

> +static const uint32_t bf_sinit[][256] = {
> +
> +  /* S-Box 0 */
> +  { 0xd1310ba6, 0x98dfb5ac, 0x2ffd72db, 0xd01adfb7, 0xb8e1afed, 0x6a267e96,
> +    0xba7c9045, 0xf12c7f99, 0x24a19947, 0xb3916cf7, 0x0801f2e2, 0x858efc16,

4-spaces indentation please, more below

> +#define      BF_ENC(X,S) \
> +     (((S[0][X>>24] + S[1][X>>16 & 0xff]) ^ S[2][(X>>8) & 0xff]) + S[3][X & 
> 0xff])

Please give the operators some room to breathe.

> +static void bf_enc(uint32_t *x, bf_key *key)
> +{
> +  uint32_t  Xl;
> +  uint32_t  Xr;
> +  uint32_t  temp;
> +  int        i;

4 space indentation

> +  for (i = 0; i < BF_ROUNDS; ++i) {
> +    Xl ^= key->p[i];
> +    Xr ^= BF_ENC(Xl,key->s);

space after comma

> +    temp = Xl;
> +    Xl = Xr;
> +    Xr = temp;

align the =

> +static void bf_setkey(const unsigned char *kp, int keybytes, bf_key *key)
> +{
> +    uint32_t data, d[2];
> +    int i, j, k;
> +
> +    memcpy(key->p, bf_pinit, sizeof(key->p));
> +    memcpy(key->s, bf_sinit, sizeof(key->s));
> +
> +    j = 0;

Merge declaration and initialization.

> +    for (i = 0; i < BF_ROUNDS + 2; ++i) {
> +        data = 0x00000000;
> +        for (k = 0; k < 4; ++k) {
> +            data = (data << 8) | kp[j];
> +            j = j + 1;
> +            if (j >= keybytes) {
> +                j = 0;
> +            }

Drop the {}

> +            key->s[i][j] = d[0];
> +            key->s[i][j + 1] = d[1];

Align the =.

> +    /* input is little-endian */
> +    d[0] = in[0] | (in[1] << 8) | (in[2] << 16) | (in[3] << 24);
> +    d[1] = in[4] | (in[5] << 8) | (in[6] << 16) | (in[7] << 24);

pointless ()

> +    out[0] = d[0] & 0xff;
> +    out[1] = (d[0] >> 8) & 0xff;
> +    out[2] = (d[0] >> 16) & 0xff;
> +    out[3] = (d[0] >> 24) & 0xff;
> +    out[4] = d[1] & 0xff;
> +    out[5] = (d[1] >> 8) & 0xff;
> +    out[6] = (d[1] >> 16) & 0xff;
> +    out[7] = (d[1] >> 24) & 0xff;

same

Right-align the numbers and the & for extra neatness.

> --- /dev/null
> +++ b/libavformat/rtmpenc.h
> @@ -0,0 +1,58 @@
> +
> +#ifndef AVFORMAT_RTMPE_H
> +#define AVFORMAT_RTMPE_H
> +
> +#include "url.h"

Add stdint.h, the header needs it.

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to