On Fri, Aug 12, 2016 at 02:59:09PM +0200, Michal Privoznik wrote:
It may happen that a developer wants to run just a specific subset of tests: tests $ VIR_TEST_RANGE=22 ../run ./virschematest This now fails miserably: ==6840== Invalid read of size 8 ==6840== at 0x4F397C0: virXMLValidatorValidate (virxml.c:1216) ==6840== by 0x402B72: testSchemaFile (virschematest.c:53) ==6840== by 0x403737: virTestRun (testutils.c:180) ==6840== by 0x402CF5: testSchemaDir (virschematest.c:98) ==6840== by 0x402EB1: testSchemaDirs (virschematest.c:131) ==6840== by 0x40314D: mymain (virschematest.c:194) ==6840== by 0x4051AF: virTestMain (testutils.c:982) ==6840== by 0x4035A9: main (virschematest.c:217) ==6840== Address 0x10 is not stack'd, malloc'd or (recently) free'd Problem is, we are trying to do two types of tests here: validate RNG schema itself, and validate XML files against RNG schemas. And the latter tries to re-use a resource allocated in the former. Therefore if the former is skipped (due to VIR_TEST_RANGE) we have to allocate the resource manually. Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> --- tests/virschematest.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)
Introduced by commit 27bdc0af, which was fixing two bugs: * a failure of virXMLValidatorInit did not set ret to -1 * the errors reported by virXMLValidatorInit were not displayed even with DEBUG on, because nothing called virDispatchError
diff --git a/tests/virschematest.c b/tests/virschematest.c
index c9cc314..1b55dad 100644
--- a/tests/virschematest.c
+++ b/tests/virschematest.c
@@ -38,6 +38,14 @@ struct testSchemaData {
const char *xml_path;
};
+static char *
+testGetSchemaPath(const char *schema)
+{
+ char *schema_path;
+ ignore_value(virAsprintf(&schema_path, "%s/docs/schemas/%s",
+ abs_topsrcdir, schema));
+ return schema_path;
+}
static int
testSchemaFile(const void *args)
@@ -120,6 +128,20 @@ testSchemaDirs(const char *schema, virXMLValidatorPtr validator, ...)
int ret = 0;
char *dir_path = NULL;
const char *dir;
+ bool freeValidator = false;
+
+ if (!validator) {
+ char *schema_path = testGetSchemaPath(schema);
+
+ if (!schema_path ||
+ !(validator = virXMLValidatorInit(schema_path))) {
+ VIR_FREE(schema_path);
This leaves ret at 0 and skips va_start (but valgrind does not complain).
+ goto cleanup;
+ }
+
+ VIR_FREE(schema_path);
+ freeValidator = true;
+ }
va_start(args, validator);
@@ -134,6 +156,8 @@ testSchemaDirs(const char *schema, virXMLValidatorPtr validator, ...)
}
cleanup:
+ if (freeValidator)
+ virXMLValidatorFree(validator);
Rather than cleaning it up conditionally, I would let the caller handle
it:
@@ -180,6 +180,12 @@ mymain(void)
data.schema = sch; \
if (virTestRun("test schema grammar file: " sch, \
testSchemaGrammar, &data) == 0) { \
+ /* initialize the validator even if the schema test \
+ * was skipped because of VIR_TEST_RANGE */ \
+ if (!data.validator && testSchemaGrammar(&data) < 0) { \
+ ret = -1; \
+ break; \
+ } \
if (testSchemaDirs(sch, data.validator, __VA_ARGS__, NULL) < 0) \
ret = -1; \
This still won't dispatch the errors from virXMLValidatorInit in case
the schema test is skipped by VIR_TEST_RANGE.
But it should not matter since the order of the tests is still altered
based on whether the schema test is skipped or not.
So ACK if you agree with my suggested diff.
If we want the test ordering independent on the schema tests, we should
not be skipping any virTestRun, initializing the schema all the way
inside testSchemaFile. I will put that on my TODO list.
Jan
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list