On Tue, 27 Sep 2016 23:27:00 +0200
Dima Ryazanov <d...@gmail.com> wrote:

> I think I actually know the point of the test.
> 
> It tries to verify that size, alloc, and data were initialized to 0, rather
> than left uninitialized - but the difficulty is that uninitialized memory
> is often already filled with 0s. So the test repeats the process a whole
> bunch of times, hoping to eventually catch non-0 uninitialized memory.

If that was the idea, still the code is so small and deterministic that I
can't see how round 2 could ever differ from round 3067.

> (That doesn't mean it's a good way to test it, though, so I have nothing
> against removing it. Something like valgrind is probably better.)

Right. Even memset(&arr, 0x57, sizeof arr) before wl_array_init() would
be much better.

Yong, do you mind if I add the memset there on top of your patch?


Thanks,
pq

> On Tue, Sep 27, 2016 at 8:03 PM, Yong Bakos <j...@humanoriented.com> wrote:
> 
> > From: Yong Bakos <yba...@humanoriented.com>
> >
> > Include wayland-util.h in addition to wayland-private.h, to be more
> > explicit
> > about where wl_array is defined.
> >
> > Remove the useless repeated testing of wl_array_init, because if it fails
> > once
> > out of thousands of iterations we're all doomed anyway.
> >
> > Signed-off-by: Yong Bakos <yba...@humanoriented.com>
> > Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com>
> > Reviewed-by: Pekka Paalanen <pekka.paala...@collabora.co.uk>
> > ---
> > v2: Add empty line (pq)
> >
> >  tests/array-test.c | 18 ++++++------------
> >  1 file changed, 6 insertions(+), 12 deletions(-)
> >
> > diff --git a/tests/array-test.c b/tests/array-test.c
> > index b0de8e6..ed6fbfb 100644
> > --- a/tests/array-test.c
> > +++ b/tests/array-test.c
> > @@ -25,24 +25,18 @@
> >
> >  #include <stdlib.h>
> >  #include <assert.h>
> > +#include "wayland-util.h"
> >  #include "wayland-private.h"
> >  #include "test-runner.h"
> >
> >  TEST(array_init)
> >  {
> > -       const int iterations = 4122; /* this is arbitrary */
> > -       int i;
> > -
> > -       /* Init array an arbitray amount of times and verify the
> > -        * defaults are sensible. */
> > +       struct wl_array array;
> >
> > -       for (i = 0; i < iterations; i++) {
> > -               struct wl_array array;
> > -               wl_array_init(&array);
> > -               assert(array.size == 0);
> > -               assert(array.alloc == 0);
> > -               assert(array.data == 0);
> > -       }
> > +       wl_array_init(&array);
> > +       assert(array.size == 0);
> > +       assert(array.alloc == 0);
> > +       assert(array.data == 0);
> >  }
> >
> >  TEST(array_release)
> > --
> > 2.7.2
> >
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >  

Attachment: pgp8Uwh9uAqRZ.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to