Re: [PATCH wayland v2 4/4] array-test: Include wayland-util.h and simplify init test
On Oct 18, 2016, at 2:05 AM, Pekka Paalanen wrote:
>
> On Tue, 27 Sep 2016 23:27:00 +0200
> Dima Ryazanov 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?
Go for it. Thanks Pekka.
yong
>
>
> Thanks,
> pq
>
>> On Tue, Sep 27, 2016 at 8:03 PM, Yong Bakos wrote:
>>
>>> From: Yong Bakos
>>>
>>> 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
>>> Reviewed-by: Eric Engestrom
>>> Reviewed-by: Pekka Paalanen
>>> ---
>>> 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
>>> #include
>>> +#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
>>>
>
signature.asc
Description: Message signed with OpenPGP using GPGMail
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland v2 4/4] array-test: Include wayland-util.h and simplify init test
On Tue, 27 Sep 2016 23:27:00 +0200
Dima Ryazanov 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 wrote:
>
> > From: Yong Bakos
> >
> > 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
> > Reviewed-by: Eric Engestrom
> > Reviewed-by: Pekka Paalanen
> > ---
> > 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
> > #include
> > +#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
> >
pgp8Uwh9uAqRZ.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland v2 4/4] array-test: Include wayland-util.h and simplify init test
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.
(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.)
On Tue, Sep 27, 2016 at 8:03 PM, Yong Bakos wrote:
> From: Yong Bakos
>
> 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
> Reviewed-by: Eric Engestrom
> Reviewed-by: Pekka Paalanen
> ---
> 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
> #include
> +#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
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
