Merge pull request #14 from estebanzimanyi/fix/const-char-return

Read a borrowed const char * return without freeing it

Author: estebanzimanyi

.github/workflows/ci.yml

@@ -38,6 +38,8 @@ jobs:
 
       - name: Run tests
         run: dotnet test MEOS.NET.sln -c Release --no-build
+        env:
+          LD_LIBRARY_PATH: ${{ steps.provision.outputs.libmeos-prefix }}/lib
 
       - name: Smoke-test the FFI
         run: dotnet run --project ExampleApp/ExampleApp.csproj -c Release --no-build

MEOS.NET.Tests/BorrowedStringTests.cs

@@ -0,0 +1,39 @@
+using MEOS.NET.Enums;
+using MEOS.NET.Types.Temporal.Number.Float;
+
+namespace MEOS.NET.Tests
+{
+    /// <summary>
+    /// The MEOS functions that answer with a `const char *` hand back a pointer
+    /// into a static table, so the caller reads it and does not free it. Freeing
+    /// it takes the process down, which is what these read.
+    /// </summary>
+    [TestClass]
+    public class BorrowedStringTests : MeosTest
+    {
+        [TestMethod]
+        public void InterpolationOfALinearSequenceIsRead()
+        {
+            TemporalFloat temp = TemporalFloat.FromString("[25.0@2024-12-06, 27.0@2024-12-07]");
+
+            Assert.AreEqual(InterpolationType.Linear, temp.Interpolation());
+        }
+
+        [TestMethod]
+        public void InterpolationOfADiscreteSequenceIsRead()
+        {
+            TemporalFloat temp = TemporalFloat.FromString("{25.0@2024-12-06, 27.0@2024-12-07}");
+
+            Assert.AreEqual(InterpolationType.Discrete, temp.Interpolation());
+        }
+
+        [TestMethod]
+        public void TheSameBorrowedNameIsReadableTwice()
+        {
+            TemporalFloat first = TemporalFloat.FromString("[25.0@2024-12-06, 27.0@2024-12-07]");
+            TemporalFloat second = TemporalFloat.FromString("[1.0@2024-12-06, 2.0@2024-12-07]");
+
+            Assert.AreEqual(first.Interpolation(), second.Interpolation());
+        }
+    }
+}

MEOS.NET.Tests/MeosTest.cs

@@ -0,0 +1,21 @@
+using MEOS.NET.Lifecycle;
+
+namespace MEOS.NET.Tests
+{
+    /// <summary>
+    /// The base every test class derives from, so each test runs against an open
+    /// MEOS session.
+    ///
+    /// MEOS keeps its session state — the timezone its text I/O reads and writes
+    /// through, and the error handler — per thread, and the test host runs a test
+    /// on whichever thread it has free. A session opened once for the assembly
+    /// therefore reaches the thread that opened it and no other: elsewhere a
+    /// timestamp is read in the machine's own timezone rather than the suite's,
+    /// and an error raised there reaches no handler at all.
+    /// </summary>
+    public abstract class MeosTest
+    {
+        [TestInitialize]
+        public void OpenMeosSession() => MEOSLifecycle.Initialize("UTC");
+    }
+}

MEOS.NET.Tests/TemporalGeometryPointTests.cs

@@ -1,4 +1,6 @@
 using MEOS.NET.Enums;
+using MEOS.NET.Errors;
+using MEOS.NET.Exceptions;
 using MEOS.NET.Helpers;
 using MEOS.NET.Internal;
 using MEOS.NET.Types.General;
@@ -123,7 +125,13 @@ public TimestampTzSpan BoundingBox()
 
         public InterpolationType Interpolation()
         {
-            var interpolationStr = MEOSExposedFunctions.temporal_interp(this._ptr);
+            // temporal_interp names one of the interpolations MEOS defines, out of
+            // its own static table, so the name is there for every temporal value.
+            var interpolationStr = MEOSExposedFunctions.temporal_interp(this._ptr)
+                ?? throw new MEOSUnspecifiedInternalException(
+                    (int)MEOSErrorCodes.UnspecifiedInternalError,
+                    MEOSErrorCodes.UnspecifiedInternalError,
+                    "MEOS named no interpolation for this temporal value");
             return EnumConverter.InterpolationTypeFromString(interpolationStr);
         }
 

MEOS.NET/Types/Temporal/Temporal.cs

@@ -133,6 +133,17 @@ def is_string_pointer(c_type: str) -> bool:
     return t in ("char *", "char**", "char *const *", "const char *")
 
 
+def is_borrowed_string(c_type: str) -> bool:
+    """A ``const char *`` return the caller does not own.
+
+    The Utf8 string marshaller frees what it is handed back, which is right for
+    the ``char *`` MEOS mallocs for the caller and fatal for the ``const char *``
+    it returns out of a static table — ``interptype_name`` hands back an element
+    of ``MEOS_INTERPTYPE_NAMES``.  A borrowed return comes back as the pointer
+    itself and is read without a free."""
+    return " ".join(c_type.split()).startswith("const char *")
+
+
 def csharp_type_for(canonical: str) -> str:
     """Translate a libclang-canonical C type to a C# type for LibraryImport signatures."""
     t = canonical.strip()
@@ -155,6 +166,8 @@ def csharp_param_type(c_type: str, canonical: str) -> str:
 
 
 def csharp_return_type(c_type: str, canonical: str) -> str:
+    if is_borrowed_string(c_type):
+        return "IntPtr"
     if is_string_pointer(c_type):
         return "string"
     return csharp_type_for(canonical)
@@ -472,6 +485,12 @@ def _copy(indent: str) -> list[str]:
 def _emit_simple_passthrough(f: dict, ext_params: str, ext_args: str, default_rt: str | None = None) -> list[str]:
     name = f["name"]
     rt = default_rt or csharp_return_type(f["returnType"]["c"], f["returnType"]["canonical"])
+    if default_rt is None and is_borrowed_string(f["returnType"]["c"]):
+        return [
+            f"        public static string? {name}({ext_params})",
+            f"            => Marshal.PtrToStringUTF8("
+            f"SafeExecution<IntPtr>(() => MEOSExternalFunctions.{name}({ext_args})));",
+        ]
     if rt == "void":
         return [
             f"        public static void {name}({ext_params})",