[FFmpeg-devel] [PATCH] Add tests for functions in hash.c

[James Almer]

On 3/7/2016 12:26 AM, NagaChaitanya Vellanki wrote:
> ---
>  libavutil/Makefile       |  1 +
>  libavutil/hash.c         | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/fate/libavutil.mak |  4 ++++
>  tests/ref/fate/hash      | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 95 insertions(+)
>  create mode 100644 tests/ref/fate/hash
> 
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 934564f..58df75a 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -186,6 +186,7 @@ TESTPROGS = adler32                                                     \
>              file                                                        \
>              fifo                                                        \
>              float_dsp                                                   \
> +            hash                                                        \
>              hmac                                                        \
>              lfg                                                         \
>              lls                                                         \
> diff --git a/libavutil/hash.c b/libavutil/hash.c
> index 7037b0d..ac35e88 100644
> --- a/libavutil/hash.c
> +++ b/libavutil/hash.c
> @@ -237,3 +237,48 @@ void av_hash_freep(AVHashContext **ctx)
>          av_freep(&(*ctx)->ctx);
>      av_freep(ctx);
>  }
> +
> +#ifdef TEST
> +// LCOV_EXCL_START
> +#define SRC_BUF_SIZE 64
> +#define DST_BUF_SIZE 512

You should use AV_HASH_MAX_SIZE for dst. If someone goes and adds a new algorithm
with a digest bigger than 64 bytes that value will change and the dst buffer here
may not be enough anymore.
AV_HASH_MAX_SIZE*4 should do it i think.

> +
> +int main(void)
> +{
> +   struct AVHashContext *ctx = NULL;
> +   int i, j;
> +   static const uint8_t src[SRC_BUF_SIZE] = { 0 };
> +   uint8_t dst[DST_BUF_SIZE];
> +   for(i = 0; i < NUM_HASHES; i++) {

Nit: Space after for.

> +       if(av_hash_alloc(&ctx, av_hash_names(i)) < 0) {

Nit: Space after if, and no need for brackets.

> +           return 1;
> +       }
> +
> +       av_hash_init(ctx);
> +       av_hash_update(ctx, src, SRC_BUF_SIZE);
> +       memset(dst, 0, DST_BUF_SIZE);

memset (or even zero initializing dst) is probably not needed at all.
hash.h doxy says hex and b64 digests are always 0-terminated, and you never
read more than av_hash_get_size(ctx) for the binary digest.

> +       av_hash_final_hex(ctx, dst, DST_BUF_SIZE);
> +       printf("%s hex: %s\n", av_hash_get_name(ctx), dst);
> +
> +       av_hash_init(ctx);
> +       av_hash_update(ctx, src, SRC_BUF_SIZE);
> +       memset(dst, 0, DST_BUF_SIZE);
> +       av_hash_final_bin(ctx, dst, DST_BUF_SIZE);
> +       printf("%s bin: ", av_hash_get_name(ctx));
> +       for(j = 0; j < av_hash_get_size(ctx); j++) {

Nit: Space after for.

> +         printf("%#x ", dst[j]);

Indentation should be four spaces.

> +       }
> +       printf("\n");
> +
> +       av_hash_init(ctx);
> +       av_hash_update(ctx, src, SRC_BUF_SIZE);
> +       memset(dst, 0, DST_BUF_SIZE);
> +       av_hash_final_b64(ctx, dst, DST_BUF_SIZE);
> +       printf("%s b64: %s\n", av_hash_get_name(ctx), dst);
> +       av_hash_freep(&ctx);
> +   }
> +   return 0;
> +}
> +
> +// LCOV_EXCL_STOP
> +#endif