diff --git a/lib/cunit/cu_pc_schema.c b/lib/cunit/cu_pc_schema.c
index c09b7e1..63e892d 100644
--- a/lib/cunit/cu_pc_schema.c
+++ b/lib/cunit/cu_pc_schema.c
@@ -54,14 +54,42 @@ test_schema_from_xml()
}
static void
-test_schema_from_xml_with_empty_field()
+test_schema_from_xml_with_empty_description()
{
PCSCHEMA *myschema = NULL;
- char *myxmlfile = "data/simple-schema-empty-field.xml";
+ char *myxmlfile = "data/simple-schema-empty-description.xml";
char *xmlstr = file_to_str(myxmlfile);
int rv = pc_schema_from_xml(xmlstr, &myschema);
- CU_ASSERT(rv == PC_SUCCESS);
+ CU_ASSERT_EQUAL(rv, PC_SUCCESS);
+
+ pc_schema_free(myschema);
+ pcfree(xmlstr);
+}
+
+static void
+test_schema_from_xml_with_no_name()
+{
+ PCSCHEMA *myschema = NULL;
+ char *myxmlfile = "data/simple-schema-no-name.xml";
+ char *xmlstr = file_to_str(myxmlfile);
+ int rv = pc_schema_from_xml(xmlstr, &myschema);
+
+ CU_ASSERT_EQUAL(rv, PC_SUCCESS);
+
+ pc_schema_free(myschema);
+ pcfree(xmlstr);
+}
+
+static void
+test_schema_from_xml_with_empty_name()
+{
+ PCSCHEMA *myschema = NULL;
+ char *myxmlfile = "data/simple-schema-empty-name.xml";
+ char *xmlstr = file_to_str(myxmlfile);
+ int rv = pc_schema_from_xml(xmlstr, &myschema);
+
+ CU_ASSERT_EQUAL(rv, PC_SUCCESS);
pc_schema_free(myschema);
pcfree(xmlstr);
@@ -131,7 +159,7 @@ test_dimension_byteoffsets()
}
static void
-test_schema_is_valid()
+test_schema_invalid_xy()
{
static PCSCHEMA *myschema = NULL;
char *xmlstr;
@@ -140,15 +168,35 @@ test_schema_is_valid()
// See https://github.com/pgpointcloud/pointcloud/issues/28
xmlstr = "1";
rv = pc_schema_from_xml(xmlstr, &myschema);
- CU_ASSERT_EQUAL(rv, PC_SUCCESS);
-
- cu_error_msg_reset();
- rv = pc_schema_is_valid(myschema);
CU_ASSERT_EQUAL(rv, PC_FAILURE);
-
- pc_schema_free(myschema);
+ CU_ASSERT_PTR_NULL(myschema);
}
+static void
+test_schema_missing_dimension()
+{
+ PCSCHEMA *myschema;
+ char *myxmlfile = "data/simple-schema-missing-dimension.xml";
+ char *xmlstr = file_to_str(myxmlfile);
+ int rv = pc_schema_from_xml(xmlstr, &myschema);
+
+ CU_ASSERT_EQUAL(rv, PC_FAILURE);
+ CU_ASSERT_PTR_NULL(myschema);
+
+ pcfree(xmlstr);
+}
+
+
+static void
+test_schema_empty()
+{
+ PCSCHEMA *myschema;
+ char *xmlstr = "";
+ int rv = pc_schema_from_xml(xmlstr, &myschema);
+
+ CU_ASSERT_EQUAL(rv, PC_FAILURE);
+ CU_ASSERT_PTR_NULL(myschema);
+}
static void
test_schema_compression(void)
@@ -161,10 +209,8 @@ static void
test_schema_clone(void)
{
int i;
- PCSCHEMA *myschema;
PCSCHEMA *clone = pc_schema_clone(schema);
hashtable *hash, *chash;
- char *xmlstr;
CU_ASSERT_EQUAL(clone->pcid, schema->pcid);
CU_ASSERT_EQUAL(clone->ndims, schema->ndims);
CU_ASSERT_EQUAL(clone->size, schema->size);
@@ -203,30 +249,90 @@ test_schema_clone(void)
}
pc_schema_free(clone);
+}
- /* See https://github.com/pgpointcloud/pointcloud/issues/66 */
- xmlstr = "1";
- i = pc_schema_from_xml(xmlstr, &myschema);
- CU_ASSERT_EQUAL(i, PC_SUCCESS);
+static void
+test_schema_clone_empty_description(void)
+{
+ PCSCHEMA *myschema, *clone;
+
+ char *myxmlfile = "data/simple-schema-empty-description.xml";
+ char *xmlstr = file_to_str(myxmlfile);
+
+ int rv = pc_schema_from_xml(xmlstr, &myschema);
+ CU_ASSERT_EQUAL(rv, PC_SUCCESS);
clone = pc_schema_clone(myschema);
+ CU_ASSERT_PTR_NOT_NULL(clone);
CU_ASSERT_EQUAL(clone->ndims, myschema->ndims);
- CU_ASSERT_EQUAL(clone->dims[0]->name, NULL);
- CU_ASSERT_EQUAL(clone->dims[0]->description, NULL);
+ CU_ASSERT_NOT_EQUAL(clone->dims[0]->name, myschema->dims[0]->name);
+ CU_ASSERT_STRING_EQUAL(clone->dims[0]->name, myschema->dims[0]->name);
+ CU_ASSERT_EQUAL(clone->dims[0]->description, myschema->dims[0]->description);
pc_schema_free(myschema);
pc_schema_free(clone);
+ pcfree(xmlstr);
+}
+
+static void
+test_schema_clone_no_name(void)
+{
+ PCSCHEMA *myschema, *clone;
+
+ /* See https://github.com/pgpointcloud/pointcloud/issues/66 */
+ char *myxmlfile = "data/simple-schema-no-name.xml";
+ char *xmlstr = file_to_str(myxmlfile);
+
+ int rv = pc_schema_from_xml(xmlstr, &myschema);
+ CU_ASSERT_EQUAL(rv, PC_SUCCESS);
+ CU_ASSERT_PTR_NOT_NULL(myschema);
+ clone = pc_schema_clone(myschema);
+ CU_ASSERT_PTR_NOT_NULL(clone);
+ CU_ASSERT_EQUAL(clone->ndims, myschema->ndims);
+ CU_ASSERT_PTR_NULL(clone->dims[0]->name);
+ CU_ASSERT_PTR_NULL(clone->dims[0]->description);
+ pc_schema_free(myschema);
+ pc_schema_free(clone);
+ pcfree(xmlstr);
+}
+
+static void
+test_schema_clone_empty_name(void)
+{
+ PCSCHEMA *myschema, *clone;
+
+ char *myxmlfile = "data/simple-schema-empty-name.xml";
+ char *xmlstr = file_to_str(myxmlfile);
+
+ int rv = pc_schema_from_xml(xmlstr, &myschema);
+ CU_ASSERT_EQUAL(rv, PC_SUCCESS);
+ CU_ASSERT_PTR_NOT_NULL(myschema);
+ clone = pc_schema_clone(myschema);
+ CU_ASSERT_PTR_NOT_NULL(clone);
+ CU_ASSERT_EQUAL(clone->ndims, myschema->ndims);
+ CU_ASSERT_PTR_NULL(clone->dims[0]->name);
+ CU_ASSERT_PTR_NULL(clone->dims[0]->description);
+ pc_schema_free(myschema);
+ pc_schema_free(clone);
+ pcfree(xmlstr);
}
/* REGISTER ***********************************************************/
CU_TestInfo schema_tests[] = {
PC_TEST(test_schema_from_xml),
- PC_TEST(test_schema_from_xml_with_empty_field),
+ PC_TEST(test_schema_from_xml_with_empty_description),
+ PC_TEST(test_schema_from_xml_with_empty_name),
+ PC_TEST(test_schema_from_xml_with_no_name),
PC_TEST(test_schema_size),
PC_TEST(test_dimension_get),
PC_TEST(test_dimension_byteoffsets),
PC_TEST(test_schema_compression),
- PC_TEST(test_schema_is_valid),
+ PC_TEST(test_schema_invalid_xy),
+ PC_TEST(test_schema_missing_dimension),
+ PC_TEST(test_schema_empty),
PC_TEST(test_schema_clone),
+ PC_TEST(test_schema_clone_empty_description),
+ PC_TEST(test_schema_clone_no_name),
+ PC_TEST(test_schema_clone_empty_name),
CU_TEST_INFO_NULL
};
diff --git a/lib/cunit/data/simple-schema-empty-field.xml b/lib/cunit/data/simple-schema-empty-description.xml
similarity index 100%
rename from lib/cunit/data/simple-schema-empty-field.xml
rename to lib/cunit/data/simple-schema-empty-description.xml
diff --git a/lib/cunit/data/simple-schema-empty-name.xml b/lib/cunit/data/simple-schema-empty-name.xml
new file mode 100644
index 0000000..319072d
--- /dev/null
+++ b/lib/cunit/data/simple-schema-empty-name.xml
@@ -0,0 +1,52 @@
+
+
+
+ 1
+ 2
+
+ uint16_t
+ 1
+
+
+ 2
+ 4
+ X coordinate as a long integer. You must use the scale and offset information of the header to determine the double value.
+ X
+ int32_t
+ 0.01
+
+
+ 3
+ 4
+ Y coordinate as a long integer. You must use the scale and offset information of the header to determine the double value.
+ Y
+ int32_t
+ 0.01
+
+
+ 4
+ 4
+ Z coordinate as a long integer. You must use the scale and offset information of the header to determine the double value.
+ Z
+ int32_t
+ 0.01
+
+
+ 5
+ 2
+ The intensity value is the integer representation of the pulse return magnitude. This value is optional and system specific. However, it should always be included if available.
+ Intensity
+ uint16_t
+ 1
+
+
+ dimensional
+
+
+
+
+
+
+ 4326
+
+
diff --git a/lib/cunit/data/simple-schema-missing-dimension.xml b/lib/cunit/data/simple-schema-missing-dimension.xml
new file mode 100644
index 0000000..08955b2
--- /dev/null
+++ b/lib/cunit/data/simple-schema-missing-dimension.xml
@@ -0,0 +1,45 @@
+
+
+
+ 1
+ 4
+ X coordinate as a long integer. You must use the scale and offset information of the header to determine the double value.
+ X
+ int32_t
+ 0.01
+
+
+ 2
+ 4
+ Y coordinate as a long integer. You must use the scale and offset information of the header to determine the double value.
+ Y
+ int32_t
+ 0.01
+
+
+ 3
+ 4
+ Z coordinate as a long integer. You must use the scale and offset information of the header to determine the double value.
+ Z
+ int32_t
+ 0.01
+
+
+ 5
+ 2
+ The intensity value is the integer representation of the pulse return magnitude. This value is optional and system specific. However, it should always be included if available.
+ Intensity
+ uint16_t
+ 1
+
+
+ dimensional
+
+
+
+
+
+
+ 4326
+
+
diff --git a/lib/cunit/data/simple-schema-no-name.xml b/lib/cunit/data/simple-schema-no-name.xml
new file mode 100644
index 0000000..832224b
--- /dev/null
+++ b/lib/cunit/data/simple-schema-no-name.xml
@@ -0,0 +1,51 @@
+
+
+
+ 1
+ 2
+ uint16_t
+ 1
+
+
+ 2
+ 4
+ X coordinate as a long integer. You must use the scale and offset information of the header to determine the double value.
+ X
+ int32_t
+ 0.01
+
+
+ 3
+ 4
+ Y coordinate as a long integer. You must use the scale and offset information of the header to determine the double value.
+ Y
+ int32_t
+ 0.01
+
+
+ 4
+ 4
+ Z coordinate as a long integer. You must use the scale and offset information of the header to determine the double value.
+ Z
+ int32_t
+ 0.01
+
+
+ 5
+ 2
+ The intensity value is the integer representation of the pulse return magnitude. This value is optional and system specific. However, it should always be included if available.
+ Intensity
+ uint16_t
+ 1
+
+
+ dimensional
+
+
+
+
+
+
+ 4326
+
+
diff --git a/lib/pc_schema.c b/lib/pc_schema.c
index a4a732b..6551abe 100644
--- a/lib/pc_schema.c
+++ b/lib/pc_schema.c
@@ -367,18 +367,6 @@ void pc_schema_check_xyzm(PCSCHEMA *s)
continue;
}
}
-
- if ( s->x_position < 0 )
- {
- pcerror("pc_schema_check_xyzm: invalid x_position '%d'", s->x_position);
- return;
- }
-
- if ( s->y_position < 0 )
- {
- pcerror("pc_schema_check_xyzm: invalid y_position '%d'", s->y_position);
- return;
- }
}
static char *
@@ -406,8 +394,8 @@ pc_schema_from_xml(const char *xml_str, PCSCHEMA **schema)
xmlDocPtr xml_doc = NULL;
xmlNodePtr xml_root = NULL;
xmlNsPtr xml_ns = NULL;
- xmlXPathContextPtr xpath_ctx;
- xmlXPathObjectPtr xpath_obj;
+ xmlXPathContextPtr xpath_ctx = NULL;
+ xmlXPathObjectPtr xpath_obj = NULL;
xmlNodeSetPtr nodes;
PCSCHEMA *s = NULL;
const char *xml_ptr = xml_str;
@@ -429,9 +417,8 @@ pc_schema_from_xml(const char *xml_str, PCSCHEMA **schema)
xml_doc = xmlReadMemory(xml_ptr, xml_size, NULL, NULL, 0);
if ( ! xml_doc )
{
- xmlCleanupParser();
pcwarn("unable to parse schema XML");
- return PC_FAILURE;
+ goto cleanup;
}
/* Capture the namespace */
@@ -441,12 +428,10 @@ pc_schema_from_xml(const char *xml_str, PCSCHEMA **schema)
/* Create xpath evaluation context */
xpath_ctx = xmlXPathNewContext(xml_doc);
- if( ! xpath_ctx )
+ if ( ! xpath_ctx )
{
- xmlFreeDoc(xml_doc);
- xmlCleanupParser();
pcwarn("unable to create new XPath context to read schema XML");
- return PC_FAILURE;
+ goto cleanup;
}
/* Register the root namespace if there is one */
@@ -455,13 +440,10 @@ pc_schema_from_xml(const char *xml_str, PCSCHEMA **schema)
/* Evaluate xpath expression */
xpath_obj = xmlXPathEvalExpression(xpath_str, xpath_ctx);
- if( ! xpath_obj )
+ if ( ! xpath_obj )
{
- xmlXPathFreeContext(xpath_ctx);
- xmlFreeDoc(xml_doc);
- xmlCleanupParser();
pcwarn("unable to evaluate xpath expression \"%s\" against schema XML", xpath_str);
- return PC_FAILURE;
+ goto cleanup;
}
/* Iterate on the dimensions we found */
@@ -475,7 +457,7 @@ pc_schema_from_xml(const char *xml_str, PCSCHEMA **schema)
for ( i = 0; i < ndims; i++ )
{
/* This is a "dimension" */
- if( nodes->nodeTab[i]->type == XML_ELEMENT_NODE )
+ if ( nodes->nodeTab[i]->type == XML_ELEMENT_NODE )
{
xmlNodePtr cur = nodes->nodeTab[i];
xmlNodePtr child;
@@ -484,7 +466,7 @@ pc_schema_from_xml(const char *xml_str, PCSCHEMA **schema)
/* These are the values of the dimension */
for ( child = cur->children; child; child = child->next )
{
- if( child->type == XML_ELEMENT_NODE && child->children != NULL)
+ if ( child->type == XML_ELEMENT_NODE && child->children != NULL)
{
char *content = (char*)(child->children->content);
char *name = (char*)(child->name);
@@ -517,30 +499,20 @@ pc_schema_from_xml(const char *xml_str, PCSCHEMA **schema)
d->size = pc_interpretation_size(d->interpretation);
/* Store the dimension in the schema */
- if ( d->position < ndims )
+ if ( d->position >= ndims )
{
- if ( s->dims[d->position] )
- {
- xmlXPathFreeObject(xpath_obj);
- xmlXPathFreeContext(xpath_ctx);
- xmlFreeDoc(xml_doc);
- xmlCleanupParser();
- pc_schema_free(s);
- pcwarn("schema dimension at position \"%d\" is declared twice", d->position + 1, ndims);
- return PC_FAILURE;
- }
- pc_schema_set_dimension(s, d);
- }
- else
- {
- xmlXPathFreeObject(xpath_obj);
- xmlXPathFreeContext(xpath_ctx);
- xmlFreeDoc(xml_doc);
- xmlCleanupParser();
- pc_schema_free(s);
pcwarn("schema dimension states position \"%d\", but number of XML dimensions is \"%d\"", d->position + 1, ndims);
- return PC_FAILURE;
+ pc_dimension_free(d);
+ goto cleanup;
+
}
+ else if ( s->dims[d->position] )
+ {
+ pcwarn("schema dimension at position \"%d\" is declared twice", d->position + 1, ndims);
+ pc_dimension_free(d);
+ goto cleanup;
+ }
+ pc_schema_set_dimension(s, d);
}
}
@@ -554,13 +526,10 @@ pc_schema_from_xml(const char *xml_str, PCSCHEMA **schema)
/* SEARCH FOR METADATA ENTRIES */
xpath_obj = xmlXPathEvalExpression(xpath_metadata_str, xpath_ctx);
- if( ! xpath_obj )
+ if ( ! xpath_obj )
{
- xmlXPathFreeContext(xpath_ctx);
- xmlFreeDoc(xml_doc);
- xmlCleanupParser();
pcwarn("unable to evaluate xpath expression \"%s\" against schema XML", xpath_metadata_str);
- return PC_FAILURE;
+ goto cleanup;
}
/* Iterate on the we find */
@@ -575,7 +544,7 @@ pc_schema_from_xml(const char *xml_str, PCSCHEMA **schema)
/* Read the metadata name and value from the node */
/* somevalue */
xmlNodePtr cur = nodes->nodeTab[i];
- if( cur->type == XML_ELEMENT_NODE && strcmp((char*)(cur->name), "Metadata") == 0 )
+ if ( cur->type == XML_ELEMENT_NODE && strcmp((char*)(cur->name), "Metadata") == 0 )
{
metadata_name = (char*)xmlGetProp(cur, (xmlChar*)"name");
metadata_value = xml_node_get_content(cur);
@@ -594,13 +563,22 @@ pc_schema_from_xml(const char *xml_str, PCSCHEMA **schema)
}
}
- xmlXPathFreeObject(xpath_obj);
+cleanup:
+ if ( s && ! pc_schema_is_valid(s) )
+ {
+ pc_schema_free(s);
+ *schema = s = NULL;
+ }
- xmlXPathFreeContext(xpath_ctx);
- xmlFreeDoc(xml_doc);
+ if ( xpath_obj )
+ xmlXPathFreeObject(xpath_obj);
+ if ( xpath_ctx )
+ xmlXPathFreeContext(xpath_ctx);
+ if ( xml_doc )
+ xmlFreeDoc(xml_doc);
xmlCleanupParser();
- return PC_SUCCESS;
+ return s ? PC_SUCCESS : PC_FAILURE;
}
uint32_t