Fix wrong property sizes from binary values (#283)

Signed-off-by: Lucas Heitzmann Gabrielli <[email protected]>

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.

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);

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) {

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) {

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) {

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) {

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},

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:

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));

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"],
         ]