[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Add test case for parsing JSON docs



On 06/30/2011 08:10 AM, Daniel P. Berrange wrote:
> While investigating some memory leaks it was unclear whether the
> JSON code correctly free'd all memory during parsing. Add a test
> case which can be run under valgrind to clearly demonstrate that
> the parser is leak free.
> 
> * tests/Makefile.am: Add 'jsontest'
> * tests/jsontest.c: A few simple JSON parsing tests
> ---
>  tests/Makefile.am |   14 +++++++
>  tests/jsontest.c  |  111 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 125 insertions(+), 0 deletions(-)
>  create mode 100644 tests/jsontest.c

Well worth having.  ACK.

However, I have a concern that will probably entail a followup patch:

> @@ -443,6 +453,10 @@ hashtest_SOURCES = \
>  	hashtest.c hashdata.h testutils.h testutils.c
>  hashtest_LDADD = $(LDADDS)
>  
> +jsontest_SOURCES = \
> +	jsontest.c jsondata.h testutils.h testutils.c
> +jsontest_LDADD = $(LDADDS)
> +
>  if WITH_LIBVIRTD
>  eventtest_SOURCES = \
>  	eventtest.c testutils.h testutils.c

We have a discrepancy here.  The rule for jsontest_SOURCES is
unconditional, even though jsontest is only compiled if HAVE_YAJL.
Meanwhile, eventtest_SOURCES is conditional on the same WITH_LIBVIRTD as
the compilation of eventtest.  Yet I don't see any EXTRA_DIST that
mentions either jsontest.c or eventtest.c if the two conditions are not met.

We need to make sure that automake includes both jsontest.c and
eventtest.c in the tarball, regardless of the configure conditionals.
We also need to make sure that automake does not try to compile
jsontest.c if HAVE_YAJL is false.  I haven't yet tested this, but think
that this implies some cleanup work is needed in tests/Makefile.am.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]