From ea6c5a7105bdde29cb2293ff41a067f7bdc314ec Mon Sep 17 00:00:00 2001 From: Lucas Heitzmann Gabrielli Date: Fri, 7 Feb 2025 16:42:24 -0300 Subject: [PATCH] Fix wrong property sizes from binary values (#283) Signed-off-by: Lucas Heitzmann Gabrielli --- CHANGELOG.md | 4 ++++ include/gdstk/property.hpp | 2 +- python/flexpath_object.cpp | 13 ++++++++++--- python/label_object.cpp | 13 ++++++++++--- python/polygon_object.cpp | 13 ++++++++++--- python/reference_object.cpp | 14 +++++++++++--- python/robustpath_object.cpp | 17 +++++++++++++---- src/library.cpp | 8 ++++---- src/property.cpp | 12 +++++++----- tests/property_test.py | 28 ++++++++++++++-------------- 10 files changed, 84 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9a3e93db..5471d3dc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased +### Fixed +- Treat string properties as binary byte arrays in OASIS. + ## 0.9.58 - 2024-11-25 ### Changed - Empty paths now give a warning when being converted to polygons or stored in GDSII/OASIS. diff --git a/include/gdstk/property.hpp b/include/gdstk/property.hpp index 9e096b7b1..b494f7c90 100644 --- a/include/gdstk/property.hpp +++ b/include/gdstk/property.hpp @@ -75,7 +75,7 @@ void set_property(Property*& properties, const char* name, const uint8_t* bytes, // Overwrite properties with the same attribute number. The NULL byte is // included in the property value. -void set_gds_property(Property*& properties, uint16_t attribute, const char* value); +void set_gds_property(Property*& properties, uint16_t attribute, const char* value, uint64_t count); uint64_t remove_property(Property*& properties, const char* name, bool all_occurences); bool remove_gds_property(Property*& properties, uint16_t attribute); diff --git a/python/flexpath_object.cpp b/python/flexpath_object.cpp index 6c7eddd56..2d259c511 100644 --- a/python/flexpath_object.cpp +++ b/python/flexpath_object.cpp @@ -1943,8 +1943,9 @@ static PyObject* flexpath_object_delete_property(FlexPathObject* self, PyObject* static PyObject* flexpath_object_set_gds_property(FlexPathObject* self, PyObject* args) { uint16_t attribute; char* value; - if (!PyArg_ParseTuple(args, "Hs:set_gds_property", &attribute, &value)) return NULL; - set_gds_property(self->flexpath->properties, attribute, value); + Py_ssize_t count; + if (!PyArg_ParseTuple(args, "Hs#:set_gds_property", &attribute, &value, &count)) return NULL; + if (count >= 0) set_gds_property(self->flexpath->properties, attribute, value, (uint64_t)count); Py_INCREF(self); return (PyObject*)self; } @@ -1957,7 +1958,13 @@ static PyObject* flexpath_object_get_gds_property(FlexPathObject* self, PyObject Py_INCREF(Py_None); return Py_None; } - return PyUnicode_FromString((char*)value->bytes); + PyObject* result = PyUnicode_FromStringAndSize((char*)value->bytes, (Py_ssize_t)value->count); + if (PyErr_Occurred()) { + Py_XDECREF(result); + PyErr_Clear(); + result = PyBytes_FromStringAndSize((char*)value->bytes, (Py_ssize_t)value->count); + } + return result; } static PyObject* flexpath_object_delete_gds_property(FlexPathObject* self, PyObject* args) { diff --git a/python/label_object.cpp b/python/label_object.cpp index 64b67affc..bed1894b7 100644 --- a/python/label_object.cpp +++ b/python/label_object.cpp @@ -169,8 +169,9 @@ static PyObject* label_object_delete_property(LabelObject* self, PyObject* args) static PyObject* label_object_set_gds_property(LabelObject* self, PyObject* args) { uint16_t attribute; char* value; - if (!PyArg_ParseTuple(args, "Hs:set_gds_property", &attribute, &value)) return NULL; - set_gds_property(self->label->properties, attribute, value); + Py_ssize_t count; + if (!PyArg_ParseTuple(args, "Hs#:set_gds_property", &attribute, &value, &count)) return NULL; + if (count >= 0) set_gds_property(self->label->properties, attribute, value, (uint64_t)count); Py_INCREF(self); return (PyObject*)self; } @@ -183,7 +184,13 @@ static PyObject* label_object_get_gds_property(LabelObject* self, PyObject* args Py_INCREF(Py_None); return Py_None; } - return PyUnicode_FromString((char*)value->bytes); + PyObject* result = PyUnicode_FromStringAndSize((char*)value->bytes, (Py_ssize_t)value->count); + if (PyErr_Occurred()) { + Py_XDECREF(result); + PyErr_Clear(); + result = PyBytes_FromStringAndSize((char*)value->bytes, (Py_ssize_t)value->count); + } + return result; } static PyObject* label_object_delete_gds_property(LabelObject* self, PyObject* args) { diff --git a/python/polygon_object.cpp b/python/polygon_object.cpp index 4fed594cb..18284a3fa 100644 --- a/python/polygon_object.cpp +++ b/python/polygon_object.cpp @@ -403,8 +403,9 @@ static PyObject* polygon_object_delete_property(PolygonObject* self, PyObject* a static PyObject* polygon_object_set_gds_property(PolygonObject* self, PyObject* args) { uint16_t attribute; char* value; - if (!PyArg_ParseTuple(args, "Hs:set_gds_property", &attribute, &value)) return NULL; - set_gds_property(self->polygon->properties, attribute, value); + Py_ssize_t count; + if (!PyArg_ParseTuple(args, "Hs#:set_gds_property", &attribute, &value, &count)) return NULL; + if (count >= 0) set_gds_property(self->polygon->properties, attribute, value, (uint64_t)count); Py_INCREF(self); return (PyObject*)self; } @@ -417,7 +418,13 @@ static PyObject* polygon_object_get_gds_property(PolygonObject* self, PyObject* Py_INCREF(Py_None); return Py_None; } - return PyUnicode_FromString((char*)value->bytes); + PyObject* result = PyUnicode_FromStringAndSize((char*)value->bytes, (Py_ssize_t)value->count); + if (PyErr_Occurred()) { + Py_XDECREF(result); + PyErr_Clear(); + result = PyBytes_FromStringAndSize((char*)value->bytes, (Py_ssize_t)value->count); + } + return result; } static PyObject* polygon_object_delete_gds_property(PolygonObject* self, PyObject* args) { diff --git a/python/reference_object.cpp b/python/reference_object.cpp index cd8a6bc67..fed790b9f 100644 --- a/python/reference_object.cpp +++ b/python/reference_object.cpp @@ -408,8 +408,10 @@ static PyObject* reference_object_delete_property(ReferenceObject* self, PyObjec static PyObject* reference_object_set_gds_property(ReferenceObject* self, PyObject* args) { uint16_t attribute; char* value; - if (!PyArg_ParseTuple(args, "Hs:set_gds_property", &attribute, &value)) return NULL; - set_gds_property(self->reference->properties, attribute, value); + Py_ssize_t count; + if (!PyArg_ParseTuple(args, "Hs#:set_gds_property", &attribute, &value, &count)) return NULL; + if (count >= 0) + set_gds_property(self->reference->properties, attribute, value, (uint64_t)count); Py_INCREF(self); return (PyObject*)self; } @@ -422,7 +424,13 @@ static PyObject* reference_object_get_gds_property(ReferenceObject* self, PyObje Py_INCREF(Py_None); return Py_None; } - return PyUnicode_FromString((char*)value->bytes); + PyObject* result = PyUnicode_FromStringAndSize((char*)value->bytes, (Py_ssize_t)value->count); + if (PyErr_Occurred()) { + Py_XDECREF(result); + PyErr_Clear(); + result = PyBytes_FromStringAndSize((char*)value->bytes, (Py_ssize_t)value->count); + } + return result; } static PyObject* reference_object_delete_gds_property(ReferenceObject* self, PyObject* args) { diff --git a/python/robustpath_object.cpp b/python/robustpath_object.cpp index 42082e6e4..674f8fc2f 100644 --- a/python/robustpath_object.cpp +++ b/python/robustpath_object.cpp @@ -1827,8 +1827,10 @@ static PyObject* robustpath_object_delete_property(RobustPathObject* self, PyObj static PyObject* robustpath_object_set_gds_property(RobustPathObject* self, PyObject* args) { uint16_t attribute; char* value; - if (!PyArg_ParseTuple(args, "Hs:set_gds_property", &attribute, &value)) return NULL; - set_gds_property(self->robustpath->properties, attribute, value); + Py_ssize_t count; + if (!PyArg_ParseTuple(args, "Hs#:set_gds_property", &attribute, &value, &count)) return NULL; + if (count >= 0) + set_gds_property(self->robustpath->properties, attribute, value, (uint64_t)count); Py_INCREF(self); return (PyObject*)self; } @@ -1841,7 +1843,13 @@ static PyObject* robustpath_object_get_gds_property(RobustPathObject* self, PyOb Py_INCREF(Py_None); return Py_None; } - return PyUnicode_FromString((char*)value->bytes); + PyObject* result = PyUnicode_FromStringAndSize((char*)value->bytes, (Py_ssize_t)value->count); + if (PyErr_Occurred()) { + Py_XDECREF(result); + PyErr_Clear(); + result = PyBytes_FromStringAndSize((char*)value->bytes, (Py_ssize_t)value->count); + } + return result; } static PyObject* robustpath_object_delete_gds_property(RobustPathObject* self, PyObject* args) { @@ -1854,7 +1862,8 @@ static PyObject* robustpath_object_delete_gds_property(RobustPathObject* self, P static PyMethodDef robustpath_object_methods[] = { {"copy", (PyCFunction)robustpath_object_copy, METH_NOARGS, robustpath_object_copy_doc}, - {"__deepcopy__", (PyCFunction)robustpath_object_deepcopy, METH_VARARGS | METH_KEYWORDS, robustpath_object_deepcopy_doc}, + {"__deepcopy__", (PyCFunction)robustpath_object_deepcopy, METH_VARARGS | METH_KEYWORDS, + robustpath_object_deepcopy_doc}, {"spine", (PyCFunction)robustpath_object_spine, METH_NOARGS, robustpath_object_spine_doc}, {"path_spines", (PyCFunction)robustpath_object_path_spines, METH_NOARGS, robustpath_object_path_spines_doc}, diff --git a/src/library.cpp b/src/library.cpp index 60e7e4a6b..ee79a0ace 100644 --- a/src/library.cpp +++ b/src/library.cpp @@ -1283,13 +1283,13 @@ Library read_gds(const char* filename, double unit, double tolerance, const Set< case GdsiiRecord::PROPVALUE: if (str[data_length - 1] != 0) str[data_length++] = 0; if (polygon) { - set_gds_property(polygon->properties, key, str); + set_gds_property(polygon->properties, key, str, data_length); } else if (path) { - set_gds_property(path->properties, key, str); + set_gds_property(path->properties, key, str, data_length); } else if (reference) { - set_gds_property(reference->properties, key, str); + set_gds_property(reference->properties, key, str, data_length); } else if (label) { - set_gds_property(label->properties, key, str); + set_gds_property(label->properties, key, str, data_length); } break; case GdsiiRecord::BGNEXTN: diff --git a/src/property.cpp b/src/property.cpp index 95d11b5c3..a134d111f 100644 --- a/src/property.cpp +++ b/src/property.cpp @@ -207,16 +207,16 @@ void set_property(Property*& properties, const char* name, const uint8_t* bytes, memcpy(value->bytes, bytes, count); } -void set_gds_property(Property*& properties, uint16_t attribute, const char* value) { +void set_gds_property(Property*& properties, uint16_t attribute, const char* value, uint64_t count) { PropertyValue* gds_attribute; PropertyValue* gds_value; Property* property = properties; for (; property; property = property->next) { if (is_gds_property(property) && property->value->unsigned_integer == attribute) { gds_value = property->value->next; - gds_value->count = strlen(value) + 1; - gds_value->bytes = (uint8_t*)reallocate(gds_value->bytes, gds_value->count); - memcpy(gds_value->bytes, value, gds_value->count); + gds_value->count = count; + gds_value->bytes = (uint8_t*)reallocate(gds_value->bytes, count); + memcpy(gds_value->bytes, value, count); return; } } @@ -226,7 +226,9 @@ void set_gds_property(Property*& properties, uint16_t attribute, const char* val gds_attribute->unsigned_integer = attribute; gds_attribute->next = gds_value; gds_value->type = PropertyType::String; - gds_value->bytes = (uint8_t*)copy_string(value, &gds_value->count); + gds_value->bytes = (uint8_t*)allocate(count); + memcpy(gds_value->bytes, value, count); + gds_value->count = count; gds_value->next = NULL; property = (Property*)allocate(sizeof(Property)); property->name = (char*)allocate(COUNT(s_gds_property_name)); diff --git a/tests/property_test.py b/tests/property_test.py index a57555eca..a21b68e30 100644 --- a/tests/property_test.py +++ b/tests/property_test.py @@ -28,8 +28,8 @@ def test_gds_properties(): obj.set_gds_property(14, "Fourth text") assert obj.get_gds_property(13) == "Third text" assert obj.properties == [ - ["S_GDS_PROPERTY", 14, b"Fourth text\x00"], - ["S_GDS_PROPERTY", 13, b"Third text\x00"], + ["S_GDS_PROPERTY", 14, b"Fourth text"], + ["S_GDS_PROPERTY", 13, b"Third text"], ] @@ -72,14 +72,14 @@ def test_properties(): def test_delete_gds_property(): def create_props(obj) -> gdstk.Reference: - obj.set_gds_property(100, "bar") + obj.set_gds_property(100, b"ba\x00r") obj.set_gds_property(101, "baz") obj.set_gds_property(102, "quux") assert obj.properties == [ - ["S_GDS_PROPERTY", 102, b"quux\x00"], - ["S_GDS_PROPERTY", 101, b"baz\x00"], - ["S_GDS_PROPERTY", 100, b"bar\x00"], + ["S_GDS_PROPERTY", 102, b"quux"], + ["S_GDS_PROPERTY", 101, b"baz"], + ["S_GDS_PROPERTY", 100, b"ba\x00r"], ] return obj @@ -94,12 +94,12 @@ def create_props(obj) -> gdstk.Reference: create_props(obj) obj.delete_gds_property(102) - assert obj.get_gds_property(100) == "bar" + assert obj.get_gds_property(100) == "ba\x00r" assert obj.get_gds_property(101) == "baz" assert obj.get_gds_property(102) is None assert obj.properties == [ - ["S_GDS_PROPERTY", 101, b"baz\x00"], - ["S_GDS_PROPERTY", 100, b"bar\x00"], + ["S_GDS_PROPERTY", 101, b"baz"], + ["S_GDS_PROPERTY", 100, b"ba\x00r"], ] for obj in ( @@ -112,12 +112,12 @@ def create_props(obj) -> gdstk.Reference: create_props(obj) obj.delete_gds_property(101) - assert obj.get_gds_property(100) == "bar" + assert obj.get_gds_property(100) == "ba\x00r" assert obj.get_gds_property(101) is None assert obj.get_gds_property(102) == "quux" assert obj.properties == [ - ["S_GDS_PROPERTY", 102, b"quux\x00"], - ["S_GDS_PROPERTY", 100, b"bar\x00"], + ["S_GDS_PROPERTY", 102, b"quux"], + ["S_GDS_PROPERTY", 100, b"ba\x00r"], ] for obj in ( @@ -134,6 +134,6 @@ def create_props(obj) -> gdstk.Reference: assert obj.get_gds_property(101) == "baz" assert obj.get_gds_property(102) == "quux" assert obj.properties == [ - ["S_GDS_PROPERTY", 102, b"quux\x00"], - ["S_GDS_PROPERTY", 101, b"baz\x00"], + ["S_GDS_PROPERTY", 102, b"quux"], + ["S_GDS_PROPERTY", 101, b"baz"], ]