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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)