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

[NagaChaitanya Vellanki]

Hi Reimar,
Will send in a new patch with the improvements for review :-).

Thank you,
Naga

On Wed, Mar 9, 2016 at 11:05 AM, Reimar Döffinger <Reimar.Doeffinger at gmx.de>
wrote:

> On Wed, Mar 09, 2016 at 10:27:29AM -0800, NagaChaitanya Vellanki wrote:
> > On Tue, Mar 8, 2016 at 5:33 PM, James Almer <jamrial at gmail.com> wrote:
> >
> > > On 3/8/2016 2:21 AM, NagaChaitanya Vellanki wrote:
> > > > ---
> > > >  libavutil/Makefile       |  1 +
> > > >  libavutil/hash.c         | 42
> ++++++++++++++++++++++++++++++++++++++++++
> > > >  tests/fate/libavutil.mak |  4 ++++
> > > >  tests/ref/fate/hash      | 45
> > > +++++++++++++++++++++++++++++++++++++++++++++
> > > >  4 files changed, 92 insertions(+)
> > > >  create mode 100644 tests/ref/fate/hash
> > >
> > > LGTM. I'll let the maintainer give the final ok and push it (CCing him
> as
> > > well).
>
> It looks fine to me, though there are a few things that could
> be improved if wanted.
> For example all-0s for src is a bit boring, just a
> couple of random numbers would be a better test.
> At 64 bytes it is also a bit short, for some hashes that
> would mean the same size as the hash, and thus some
> parts of the algorithm would never ever run.
> A power-of-two size also means the padding special-cases
> are not tested.
> Something like e.g. 133 bytes might be a better test
> (assuming all our hashes support that as input?
> I actually have no idea who is responsible for padding).
> As we are testing things, changing the memset to
> not 0 but e.g. 'a' would ensure that we would notice
> if someone broke then 0-termination of the string,
> and it should then also be run before the av_hash_final_b64.
> But regardless of that it is great to have a test,
> these are just some improvement ideas that also
> can be added later.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>