neuronapi: rework nrn_setpointer to the pop idiom

Per review: rather than naming one kind of source (a range variable at
sec/x), take the source pointer from the top of the stack. The caller
pushes it any way the stack supports (e.g. nrn_rangevar_push), then
nrn_setpointer_pop consumes it and assigns it to the POINTER's dparam
slot. The pushed pointer is popped even on the error paths, so the stack
stays balanced; the test now pushes-then-calls and asserts that hygiene.

Also switch the error-buffer copy to snprintf (guaranteed NUL-termination,
clears the SonarCloud strncpy flag).

Author: ec1dev

docs/capi.rst

@@ -668,17 +668,20 @@ Segments
             seg.g_pas = 0.001  # S/cm²
 
 
-.. c:function:: int nrn_setpointer(Symbol* pointer_sym, Section* sec, double x, Symbol* src_sym, Section* src_sec, double src_x, char* error_msg, size_t error_msg_size)
+.. c:function:: int nrn_setpointer_pop(Symbol* pointer_sym, Section* sec, double x, char* error_msg, size_t error_msg_size)
 
-    Wire an NMODL ``POINTER`` variable to a source variable's storage.
+    Wire an NMODL ``POINTER`` variable to the source pointer on top of the stack.
+
+    The source is whatever pointer the caller has pushed, e.g. with
+    :c:func:`nrn_rangevar_push`. Pushing the source rather than naming it lets
+    this single function accept a pointer obtained any way the stack supports,
+    instead of enumerating source kinds. The pushed pointer is consumed (popped)
+    even on the error paths, so the stack is left balanced.
 
     :param pointer_sym: Symbol of the ``POINTER`` range variable to wire (the
         target).
     :param sec: Section of the mechanism instance owning the POINTER.
     :param x: Normalized position (0.0 to 1.0) of that instance.
-    :param src_sym: Symbol of the source range variable.
-    :param src_sec: Section of the source variable.
-    :param src_x: Normalized position of the source variable.
     :param error_msg: Buffer filled with a message on failure (may be ``NULL``).
     :param error_msg_size: Size of ``error_msg``.
     :returns: 0 on success; nonzero on error, with ``error_msg`` populated when
@@ -698,7 +701,8 @@ Segments
         Symbol* vgap = nrn_symbol("vgap_halfgap");
         Symbol* v = nrn_symbol("v");
         char err[256];
-        if (nrn_setpointer(vgap, cell1, 0.5, v, cell2, 0.5, err, sizeof(err))) {
+        nrn_rangevar_push(v, cell2, 0.5);  // push the source pointer
+        if (nrn_setpointer_pop(vgap, cell1, 0.5, err, sizeof(err))) {
             fprintf(stderr, "setpointer failed: %s\n", err);
         }
 

src/nrniv/neuronapi.cpp

@@ -230,24 +230,25 @@ void nrn_rangevar_push(Symbol* sym, Section* sec, double x) {
     hoc_push(nrn_rangepointer(sec, sym, x));
 }
 
-int nrn_setpointer(Symbol* pointer_sym,
-                   Section* sec,
-                   double x,
-                   Symbol* src_sym,
-                   Section* src_sec,
-                   double src_x,
-                   char* error_msg,
-                   size_t error_msg_size) {
+int nrn_setpointer_pop(Symbol* pointer_sym,
+                       Section* sec,
+                       double x,
+                       char* error_msg,
+                       size_t error_msg_size) {
     if (error_msg && error_msg_size > 0) {
         error_msg[0] = '\0';
     }
     auto fail = [&](const char* msg) {
         if (error_msg && error_msg_size > 0) {
-            strncpy(error_msg, msg, error_msg_size - 1);
-            error_msg[error_msg_size - 1] = '\0';
+            std::snprintf(error_msg, error_msg_size, "%s", msg);
         }
         return 1;
     };
+    // The source pointer is whatever the caller pushed (e.g. via
+    // nrn_rangevar_push), which reuses every existing way of obtaining one. Pop
+    // it first, before the validations below, so the stack stays balanced even
+    // on the error paths. It is the same data handle a dparam slot holds.
+    neuron::container::data_handle<double> src = hoc_pop_handle<double>();
     // Only a mechanism POINTER range variable can be a setpointer target; the
     // slot it wires lives in the mechanism's dparam array (mirrors the guard in
     // nrn_pointer_assign / nrnpy_nrn.cpp).
@@ -266,11 +267,10 @@ int nrn_setpointer(Symbol* pointer_sym,
     if (!prop) {
         return fail("the POINTER's mechanism is not present at the target segment");
     }
-    // Wire the POINTER to the source variable's storage. nrn_rangepointer yields
-    // a data handle (stable across data permutation), which is exactly what the
-    // dparam slot stores -- the same assignment nrn_pointer_assign performs,
-    // without the PyObject* source.
-    prop->dparam[pointer_sym->u.rng.index] = nrn_rangepointer(src_sec, src_sym, src_x);
+    // Wire the POINTER to the popped source handle (stable across data
+    // permutation), the same assignment nrn_pointer_assign performs without the
+    // PyObject* source.
+    prop->dparam[pointer_sym->u.rng.index] = src;
     return 0;
 }
 

src/nrniv/neuronapi.h

@@ -72,14 +72,11 @@ int nrn_segment_node_index(Section* sec, double x);
 void nrn_rangevar_push(Symbol* sym, Section* sec, double x);
 double nrn_rangevar_get(Symbol* sym, Section* sec, double x);
 void nrn_rangevar_set(Symbol* sym, Section* sec, double x, double value);
-int nrn_setpointer(Symbol* pointer_sym,
-                   Section* sec,
-                   double x,
-                   Symbol* src_sym,
-                   Section* src_sec,
-                   double src_x,
-                   char* error_msg,
-                   size_t error_msg_size);
+int nrn_setpointer_pop(Symbol* pointer_sym,
+                       Section* sec,
+                       double x,
+                       char* error_msg,
+                       size_t error_msg_size);
 
 /****************************************
  * Functions, objects, and the stack

test/api/CMakeLists.txt

@@ -23,7 +23,7 @@ foreach(
                                             "CURRENT_SOURCE_DIR=${CMAKE_CURRENT_SOURCE_DIR}")
 endforeach()
 
-# The nrn_setpointer test needs a mechanism with a POINTER, which no built-in provides. Generate one
+# The nrn_setpointer_pop test needs a mechanism with a POINTER, which no built-in provides. Generate one
 # from ptrtest.mod with nocmodl and compile it into the test executable, the same codegen path nrniv
 # uses for its built-in mechanisms. NRN_NOCMODL_SANITIZER_ENVIRONMENT disables LeakSanitizer for the
 # nocmodl run (nocmodl leaks, which would otherwise fail the generation step under an asan build),

test/api/ptrtest.mod

@@ -1,8 +1,8 @@
-TITLE minimal POINTER density mechanism for the nrn_setpointer api test
+TITLE minimal POINTER density mechanism for the nrn_setpointer_pop api test
 
 : A density mechanism with a single POINTER (src) and a plain range variable
 : (out). INITIAL copies the pointed-to value into out, so a test can wire src
-: with nrn_setpointer, call finitialize, and read out back to confirm the
+: with nrn_setpointer_pop, call finitialize, and read out back to confirm the
 : POINTER now aliases the source's storage.
 
 NEURON {

test/api/setpointer.cpp

@@ -1,11 +1,13 @@
 // NOTE: this assumes neuronapi.h is on your CPLUS_INCLUDE_PATH
-// Exercises nrn_setpointer, which wires an NMODL POINTER range variable to a
-// source variable's storage (the Python-free form of the HOC `setpointer`
-// statement / nrn_pointer_assign). The ptrtest density mechanism copies its
-// POINTER `src` into the range variable `out` in INITIAL, so after wiring
-// src -> a source double and calling finitialize, reading out confirms the
-// POINTER now aliases the source. modl_reg registers ptrtest (its generated
-// _ptrtest_reg is compiled into this test), the way nrniv registers built-ins.
+// Exercises nrn_setpointer_pop, which wires an NMODL POINTER range variable to
+// the source pointer the caller has pushed onto the stack (the Python-free form
+// of the HOC `setpointer` statement / nrn_pointer_assign). Pushing the source
+// rather than naming it reuses every existing way of obtaining a pointer. The
+// ptrtest density mechanism copies its POINTER `src` into the range variable
+// `out` in INITIAL, so after wiring src -> a source double and calling
+// finitialize, reading out confirms the POINTER now aliases the source.
+// modl_reg registers ptrtest (its generated _ptrtest_reg is compiled into this
+// test), the way nrniv registers built-ins.
 #include <array>
 #include <cstring>
 #include <iostream>
@@ -58,10 +60,12 @@ int main(void) {
     ok &= check(src_sym != nullptr && out_sym != nullptr && v_sym != nullptr,
                 "ptrtest range-variable symbols resolve");
 
-    // Wire dend(0.5).ptrtest.src -> soma(0.5).v
+    // Wire dend(0.5).ptrtest.src -> soma(0.5).v. Push the source pointer, then
+    // let setpointer_pop consume it and assign it to the POINTER's dparam slot.
     char err[256];
-    int rc = nrn_setpointer(src_sym, dend, 0.5, v_sym, soma, 0.5, err, sizeof(err));
-    ok &= check(rc == 0, "nrn_setpointer succeeds for a valid POINTER wiring");
+    nrn_rangevar_push(v_sym, soma, 0.5);
+    int rc = nrn_setpointer_pop(src_sym, dend, 0.5, err, sizeof(err));
+    ok &= check(rc == 0, "nrn_setpointer_pop succeeds for a valid POINTER wiring");
     if (rc != 0) {
         cerr << "  error_msg: " << err << endl;
     }
@@ -76,15 +80,25 @@ int main(void) {
              "POINTER reads the wired source's value after finitialize");
 
     // Error path 1: a non-POINTER target (out is a plain RANGE var) is rejected
-    // with a nonzero return and a message, not a crash.
-    int rc_bad = nrn_setpointer(out_sym, dend, 0.5, v_sym, soma, 0.5, err, sizeof(err));
+    // with a nonzero return and a message, not a crash. The pushed source is
+    // still consumed (popped) before the validation fails, so the stack stays
+    // balanced.
+    nrn_rangevar_push(v_sym, soma, 0.5);
+    int rc_bad = nrn_setpointer_pop(out_sym, dend, 0.5, err, sizeof(err));
     ok &= check(rc_bad != 0, "non-POINTER target is rejected");
     ok &= check(err[0] != '\0', "rejection fills the error message");
 
     // Error path 2: the POINTER's mechanism is absent at the target segment
     // (soma has no ptrtest). Rejected, not a crash.
-    int rc_absent = nrn_setpointer(src_sym, soma, 0.5, v_sym, soma, 0.5, err, sizeof(err));
+    nrn_rangevar_push(v_sym, soma, 0.5);
+    int rc_absent = nrn_setpointer_pop(src_sym, soma, 0.5, err, sizeof(err));
     ok &= check(rc_absent != 0, "POINTER target on a segment without the mechanism is rejected");
 
+    // Stack hygiene: every call above (success and error paths) consumed
+    // exactly the one source it was given. Push a sentinel and confirm it comes
+    // straight back, proving nothing was left on or over-popped from the stack.
+    nrn_double_push(17.0);
+    ok &= eq(nrn_double_pop(), 17.0, "the stack is balanced after the setpointer_pop calls");
+
     return ok ? 0 : 1;
 }