7:59 a.m.
It's true that rewriting this into C made this test run faster.
But maybe too much to my taste.
Michal Privoznik (2):
virschematest: Initialize @data
virschematest: Make sure that validator is initialized
tests/virschematest.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
--
2.8.4
7:59 a.m.
New subject: [libvirt] [PATCH 1/2] virschematest: Initialize @data
==8630== Invalid read of size 8
==8630== at 0x4EA4F0F: virFree (viralloc.c:582)
==8630== by 0x4F398F0: virXMLValidatorFree (virxml.c:1257)
==8630== by 0x40305C: mymain (virschematest.c:191)
==8630== by 0x405159: virTestMain (testutils.c:982)
==8630== by 0x403553: main (virschematest.c:215)
==8630== Address 0xcd72243 is 131 bytes inside a block of size 177 free'd
==8630== at 0x4C2B1F0: free (vg_replace_malloc.c:473)
==8630== by 0x4EA4F19: virFree (viralloc.c:582)
==8630== by 0x4ED0973: virFindFileInPath (virfile.c:1646)
==8630== by 0x405149: virTestMain (testutils.c:980)
==8630== by 0x403553: main (virschematest.c:215)
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/virschematest.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/virschematest.c b/tests/virschematest.c
index 45960ba..c9cc314 100644
--- a/tests/virschematest.c
+++ b/tests/virschematest.c
@@ -173,6 +173,8 @@ mymain(void)
int ret = 0;
struct testSchemaFileData data;
+ memset(&data, 0, sizeof(data));
+
#define DO_TEST(sch, ...) \
do { \
data.schema = sch; \
--
2.8.4
8:53 a.m.
New subject: [libvirt] [PATCH 1/2] virschematest: Initialize @data
On Fri, Aug 12, 2016 at 02:59:08PM +0200, Michal Privoznik wrote:
==8630== Invalid read of size 8
==8630== at 0x4EA4F0F: virFree (viralloc.c:582)
==8630== by 0x4F398F0: virXMLValidatorFree (virxml.c:1257)
==8630== by 0x40305C: mymain (virschematest.c:191)
==8630== by 0x405159: virTestMain (testutils.c:982)
==8630== by 0x403553: main (virschematest.c:215)
==8630== Address 0xcd72243 is 131 bytes inside a block of size 177 free'd
==8630== at 0x4C2B1F0: free (vg_replace_malloc.c:473)
==8630== by 0x4EA4F19: virFree (viralloc.c:582)
==8630== by 0x4ED0973: virFindFileInPath (virfile.c:1646)
==8630== by 0x405149: virTestMain (testutils.c:980)
==8630== by 0x403553: main (virschematest.c:215)
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/virschematest.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/virschematest.c b/tests/virschematest.c
index 45960ba..c9cc314 100644
--- a/tests/virschematest.c
+++ b/tests/virschematest.c
@@ -173,6 +173,8 @@ mymain(void)
int ret = 0;
struct testSchemaFileData data;
Could be just data = {0};
+ memset(&data, 0, sizeof(data));
+
ACK either way.
Jan
7:59 a.m.
New subject: [libvirt] [PATCH 2/2] virschematest: Make sure that validator is initialized
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(a)redhat.com>
---
tests/virschematest.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
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);
+ 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);
VIR_FREE(dir_path);
va_end(args);
return ret;
@@ -152,8 +176,7 @@ testSchemaGrammar(const void *opaque)
char *schema_path;
int ret = -1;
- if (virAsprintf(&schema_path, "%s/docs/schemas/%s",
- abs_topsrcdir, data->schema) < 0)
+ if (!(schema_path = testGetSchemaPath(data->schema)))
return -1;
if (!(data->validator = virXMLValidatorInit(schema_path)))
--
2.8.4
9:35 a.m.
New subject: [libvirt] [PATCH 2/2] virschematest: Make sure that validator is initialized
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(a)redhat.com>
---
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
3024
days inactive
3024
days old
4 comments
2 participants
participants (2)
-
Ján Tomko -
Michal Privoznik