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@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list