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

[NagaChaitanya Vellanki]

Please review the latest patch. the DST_BUF_SIZE is not AV_HASH_MAX_SIZE *
8 since AV_HASH_MAX_SIZE is 64.

Thank you,
Naga

On Sun, Mar 6, 2016 at 9:45 PM, James Almer <jamrial at gmail.com> wrote:

> 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
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>