From f09ed387531cd5185e0c51a60018b144593f4fbf Mon Sep 17 00:00:00 2001
From: David Hendriks <davidhendriks93@gmail.com>
Date: Wed, 7 Apr 2021 20:21:59 +0100
Subject: [PATCH] implemented pycapsules to handle the passing of pointers
 better

---
 include/binary_c_python.h |  12 +--
 src/binary_c_python.c     | 205 +++++++++++++++++---------------------
 2 files changed, 99 insertions(+), 118 deletions(-)

diff --git a/include/binary_c_python.h b/include/binary_c_python.h
index c66ae41f2..8d0cfec88 100644
--- a/include/binary_c_python.h
+++ b/include/binary_c_python.h
@@ -10,8 +10,8 @@
 /* Binary_c's python API prototypes */
 int run_system(char * argstring,
                long int custom_logging_func_memaddr,
-               long int store_memaddr,
-               long int persistent_data_memaddr,
+               struct libbinary_c_store_t * store,
+               struct persistent_data_t * persistent_data,
                int write_logfile,
                int population,
                char ** const buffer,
@@ -51,20 +51,20 @@ int return_minimum_orbit_for_RLOF(char * argstring,
 /* Functions to handle memory                                          */
 /* =================================================================== */
 
-long int return_store_memaddr(char ** const buffer,
+struct libbinary_c_store_t * return_store_memaddr(char ** const buffer,
                char ** const error_buffer,
                size_t * const nbytes);
 
-long int return_persistent_data_memaddr(char ** const buffer,
+struct persistent_data_t * return_persistent_data_memaddr(char ** const buffer,
                char ** const error_buffer,
                size_t * const nbytes);
 
-int free_persistent_data_memaddr_and_return_json_output(long int persistent_data_memaddr,
+int free_store_memaddr(struct libbinary_c_store_t * store,
                char ** const buffer,
                char ** const error_buffer,
                size_t * const nbytes);
 
-int free_store_memaddr(long int store_memaddr,
+int free_persistent_data_memaddr_and_return_json_output(struct persistent_data_t * persistent_data,
                char ** const buffer,
                char ** const error_buffer,
                size_t * const nbytes);
diff --git a/src/binary_c_python.c b/src/binary_c_python.c
index 1e2737669..94ed0a620 100644
--- a/src/binary_c_python.c
+++ b/src/binary_c_python.c
@@ -196,19 +196,44 @@ static PyObject* binary_c_run_system(PyObject *self, PyObject *args, PyObject *k
     /* set vars and default values for some*/
     char *argstring;
     long int custom_logging_func_memaddr = -1;
-    long int store_memaddr = -1;
-    long int persistent_data_memaddr = -1;
+    PyObject *  store_capsule = NULL;
+    PyObject * persistent_data_capsule = NULL;
     int write_logfile = 0;
     int population = 0;
 
-    // By using the keywords argument it scans over the given set of kwargs, but if they are not given then the default value is used
     /* Parse the input tuple */
-    if(!PyArg_ParseTupleAndKeywords(args, kwargs, "s|lllii", keywords, &argstring, &custom_logging_func_memaddr, &store_memaddr, &persistent_data_memaddr, &write_logfile, &population))
+    // By using the keywords argument it scans over the given set of kwargs, but if they are not given then the default value is used
+    if(!PyArg_ParseTupleAndKeywords(args, kwargs, "s|lOOii", keywords, &argstring, &custom_logging_func_memaddr, &store_capsule, &persistent_data_capsule, &write_logfile, &population))
     {
         return NULL;
     }
 
-    // printf("Input persistent_Data_memaddr: %lu\n", persistent_data_memaddr);
+    // TODO: Build in checks for all the capsules
+
+    /* Unpack the capsules */
+    // Persistent data
+    struct libbinary_c_persistent_data_t * persistent_data = NULL;
+    if (persistent_data_capsule != NULL)
+    {
+        if (PyCapsule_IsValid(persistent_data_capsule, "PERSISTENT_DATA"))
+        {
+            if (!(persistent_data = (struct libbinary_c_persistent_data_t *) PyCapsule_GetPointer(persistent_data_capsule, "PERSISTENT_DATA")))
+                return NULL;   
+            debug_printf("Unpacked persistent_data pointer %p from capsule\n", persistent_data);                
+        }
+    }
+
+    // Store
+    struct libbinary_c_store_t * store = NULL;
+    if (store_capsule != NULL)
+    {
+        if (PyCapsule_IsValid(store_capsule, "STORE"))
+        {
+            if (!(store = (struct libbinary_c_store_t *) PyCapsule_GetPointer(store_capsule, "STORE")))
+                return NULL;   
+            debug_printf("Unpacked store pointer %p from capsule\n", store_capsule);            
+        }
+    }
 
     /* Call c-function */
     char * buffer;
@@ -216,8 +241,8 @@ static PyObject* binary_c_run_system(PyObject *self, PyObject *args, PyObject *k
     size_t nbytes;
     int out MAYBE_UNUSED = run_system(argstring,                    // the argstring
                                       custom_logging_func_memaddr,  // memory adress for the function for custom logging
-                                      store_memaddr,                // memory adress for the store object
-                                      persistent_data_memaddr,      // memory adress for the persistent data
+                                      store,                // TODO: change. memory adress for the store object
+                                      persistent_data,      // TODO: change. memory adress for the persistent data
                                       write_logfile,                // boolean for whether to write the logfile
                                       population,                   // boolean for whether this is part of a population.
                                       &buffer,
@@ -411,7 +436,7 @@ static PyObject* binary_c_return_store_memaddr(PyObject *self, PyObject *args)
     char * buffer;
     char * error_buffer;
     size_t nbytes;
-    long int out MAYBE_UNUSED = return_store_memaddr(&buffer,
+    struct libbinary_c_store_t * store MAYBE_UNUSED = return_store_memaddr(&buffer,
                                       &error_buffer,
                                       &nbytes);
 
@@ -419,7 +444,8 @@ static PyObject* binary_c_return_store_memaddr(PyObject *self, PyObject *args)
     PyObject * return_string MAYBE_UNUSED = Py_BuildValue("s", buffer);
     PyObject * return_error_string MAYBE_UNUSED = Py_BuildValue("s", error_buffer);
 
-    PyObject * store_memaddr = Py_BuildValue("l", out);
+    debug_printf("Packing up store pointer %p into capsule\n", store);
+    PyObject * store_memaddr_capsule = PyCapsule_New(store, "STORE", NULL);
 
     if(error_buffer != NULL && strlen(error_buffer)>0)
     {
@@ -433,7 +459,7 @@ static PyObject* binary_c_return_store_memaddr(PyObject *self, PyObject *args)
     Safe_free(buffer);
     Safe_free(error_buffer);
 
-    return store_memaddr;
+    return store_memaddr_capsule;
 }
 
 static PyObject* binary_c_return_persistent_data_memaddr(PyObject *self, PyObject *args)
@@ -442,7 +468,7 @@ static PyObject* binary_c_return_persistent_data_memaddr(PyObject *self, PyObjec
     char * buffer;
     char * error_buffer;
     size_t nbytes;
-    long int out MAYBE_UNUSED = return_persistent_data_memaddr(&buffer,
+    struct libbinary_c_persistent_data_t * persistent_data MAYBE_UNUSED = return_persistent_data_memaddr(&buffer,
                                       &error_buffer,
                                       &nbytes);
 
@@ -450,8 +476,8 @@ static PyObject* binary_c_return_persistent_data_memaddr(PyObject *self, PyObjec
     PyObject * return_string MAYBE_UNUSED = Py_BuildValue("s", buffer);
     PyObject * return_error_string MAYBE_UNUSED = Py_BuildValue("s", error_buffer);
 
-    PyObject * persistent_data_memaddr = Py_BuildValue("l", out);
-    // printf("persistent_data_memaddr: %ld\n", out);
+    debug_printf("Packing up persistent_data pointer %p into capsule\n", persistent_data);
+    PyObject * persistent_data_memaddr_capsule = PyCapsule_New(persistent_data, "PERSISTENT_DATA", NULL);
 
     if(error_buffer != NULL && strlen(error_buffer)>0)
     {
@@ -465,7 +491,7 @@ static PyObject* binary_c_return_persistent_data_memaddr(PyObject *self, PyObjec
     Safe_free(buffer);
     Safe_free(error_buffer);
 
-    return persistent_data_memaddr;
+    return persistent_data_memaddr_capsule;
 }
 
 /* Memory freeing functions */
@@ -473,19 +499,28 @@ static PyObject* binary_c_free_persistent_data_memaddr_and_return_json_output(Py
 {
     /* Python binding that calls the c function that free's the persistent data memory and prints out the json */
 
-    long int persistent_data_memaddr = -1;
-
-    /* Parse the input tuple */
-    if(!PyArg_ParseTuple(args, "l", &persistent_data_memaddr))
+    // Unpack the input
+    PyObject *persistent_data_memaddr_capsule = NULL;
+    if (!PyArg_ParseTuple(args, "O", &persistent_data_memaddr_capsule))
     {
-        return NULL;
+        fprintf(stderr,
+                "Error (in function: binary_c_free_persistent_data_memaddr_and_return_json_output): Got a bad input\n");
+        printf("Error (in function: binary_c_free_persistent_data_memaddr_and_return_json_output): Got a bad input\n");
+        return NULL; // Add message for input        
     }
 
+    // Unpack the capsule
+    struct libbinary_c_persistent_data_t * persistent_data = NULL;
+    if (!(persistent_data = (struct libbinary_c_persistent_data_t *) PyCapsule_GetPointer(persistent_data_memaddr_capsule, "PERSISTENT_DATA")))
+        return NULL;   
+    debug_printf("Unpacked persistent_data pointer %p from capsule\n", persistent_data);
+
+    // Interface with binary_c API
     char * buffer;
     char * error_buffer;
     size_t nbytes;
 
-    long int out MAYBE_UNUSED = free_persistent_data_memaddr_and_return_json_output(persistent_data_memaddr,
+    int out MAYBE_UNUSED = free_persistent_data_memaddr_and_return_json_output(persistent_data,
                                       &buffer,
                                       &error_buffer,
                                       &nbytes);
@@ -512,22 +547,28 @@ static PyObject* binary_c_free_persistent_data_memaddr_and_return_json_output(Py
 static PyObject* binary_c_free_store_memaddr(PyObject *self, PyObject *args)
 {
     /* Python binding that calls the c function that free's the store memory */
-    long int store_memaddr = -1;
+    char * buffer;
+    char * error_buffer;
+    size_t nbytes;
 
-    /* Parse the input tuple */
-    if(!PyArg_ParseTuple(args, "l", &store_memaddr))
+    PyObject *store_memaddr_capsule = NULL;
+    if (!PyArg_ParseTuple(args, "O", &store_memaddr_capsule))
     {
         fprintf(stderr,
                 "Error (in function: binary_c_free_store_memaddr): Got a bad input\n");
         printf("Error (in function: binary_c_free_store_memaddr): Got a bad input\n");
-        return NULL;
+        return NULL; // Add message for input        
     }
+    // TODO: Add checks for validity of capsule
 
-    char * buffer;
-    char * error_buffer;
-    size_t nbytes;
 
-    long int out MAYBE_UNUSED = free_store_memaddr(store_memaddr,
+    // Unpack the capsule
+    struct libbinary_c_store_t * store = NULL;
+    if (!(store = (struct libbinary_c_store_t *) PyCapsule_GetPointer(store_memaddr_capsule, "STORE")))
+        return NULL;   
+    debug_printf("Unpacked store pointer %p from capsule\n", store);
+
+    int out MAYBE_UNUSED = free_store_memaddr(store,
                                       &buffer,
                                       &error_buffer,
                                       &nbytes);
@@ -551,7 +592,6 @@ static PyObject* binary_c_free_store_memaddr(PyObject *self, PyObject *args)
     Py_RETURN_NONE;
 }
 
-
 static PyObject* binary_c_test_func(PyObject *self, PyObject *args)
 {
     // function to see if we can access the stability string
@@ -615,8 +655,8 @@ TODO: Describe each input
 */
 int run_system(char * argstring,
                long int custom_logging_func_memaddr,
-               long int store_memaddr,
-               long int persistent_data_memaddr,
+               struct libbinary_c_store_t * store,
+               struct persistent_data_t * persistent_data,
                int write_logfile,
                int population,
                char ** const buffer,
@@ -625,34 +665,8 @@ int run_system(char * argstring,
 {
     /* memory for system */
     struct libbinary_c_stardata_t *stardata = NULL;
-
-    // Store:
-    /* Check the value of the store_memaddr */
-    struct libbinary_c_store_t *store;
-    if(store_memaddr != -1)
-    {
-        // load the store from the integer that has been passed
-        store = (void*)store_memaddr;
-    }
-    else
-    {
-        // struct libbinary_c_store_t * store = NULL;
-        store = NULL;
-    }
-
-    // persistent_data:
-    struct libbinary_c_persistent_data_t *persistent_data;
-    if(persistent_data_memaddr != -1)
-    {
-        // load the persistent data from the long int that has been passed
-        persistent_data = (void*)persistent_data_memaddr;
-        debug_printf("Took long int memaddr %ld and loaded it to %p\n", persistent_data_memaddr, (void*)&persistent_data);
-    }
-    else
-    {
-        persistent_data = NULL;
-        debug_printf("persistent_data memory adress was -1, now setting it to NULL\n");
-    }
+    // store can be NULL, but could be a valid pointer to a store
+    // persistent_data can be NULL, but could be a valid pointer to a persistent_data
 
     /* Set up new system */
     binary_c_new_system(&stardata,          // stardata
@@ -702,15 +716,15 @@ int run_system(char * argstring,
 
     /* Determine whether to free the store memory adress*/
     Boolean free_store = FALSE;
-    if (store_memaddr == -1)
+    if (store == NULL)
     {
-	debug_printf("Decided to free the store memaddr\n");
+        debug_printf("Decided to free the store memaddr\n");
         free_store = TRUE;
     }
 
     /* Determine whether to free the persistent data memory adress*/
     Boolean free_persistent_data = FALSE;
-    if (persistent_data_memaddr == -1)
+    if (persistent_data == NULL)
     {
         debug_printf("Decided to free the persistent_data memaddr\n");
         free_persistent_data = TRUE;
@@ -998,9 +1012,8 @@ int return_minimum_orbit_for_RLOF(char * argstring,
 /* Functions to set up memory                                          */
 /* =================================================================== */
 
-
 // TODO: modify functon with pycapsules
-long int return_store_memaddr(char ** const buffer,
+struct libbinary_c_store_t * return_store_memaddr(char ** const buffer,
                char ** const error_buffer,
                size_t * const nbytes)
 {
@@ -1037,25 +1050,19 @@ long int return_store_memaddr(char ** const buffer,
         TRUE                        // free_persistent
     );
 
-    /* convert the pointer */ 
-    uintptr_t store_memaddr_int = (uintptr_t)store; // C Version converting ptr to int
-    debug_printf("store is at address: %p store_memaddr_int: %ld\n", (void*)&store, store_memaddr_int);
-
     /* Return the memaddr as an int */
-    return store_memaddr_int;
+    debug_printf("Returning store pointer %p\n", store);
+    return store;
 }
 
-
-
-// TODO: modify functon with pycapsules
-long int return_persistent_data_memaddr(char ** const buffer,
+struct persistent_data_t * return_persistent_data_memaddr(char ** const buffer,
                char ** const error_buffer,
                size_t * const nbytes)
 {
     /* Function to allocate the persistent_data_memaddr */
     struct libbinary_c_stardata_t *stardata = NULL;
     struct libbinary_c_store_t * store = NULL;
-    struct libbinary_c_persistent_data_t * persistent_data = NULL; 
+    struct persistent_data_t * persistent_data = NULL;
     char * empty_str = "";
 
     /* Set up new system */
@@ -1068,16 +1075,15 @@ long int return_persistent_data_memaddr(char ** const buffer,
                         -1                  // argc
     );
 
+    // uintptr_t persistent_data = (uintptr_t)stardata->persistent_data;
+    persistent_data = stardata->persistent_data;
+
     /* get buffer pointer */
     binary_c_buffer_info(stardata, buffer, nbytes);
     
     /* get error buffer pointer */
     binary_c_error_buffer(stardata, error_buffer);
         
-    /* convert the pointer */
-    uintptr_t persistent_data_memaddr_int = (uintptr_t)stardata->persistent_data; // C Version converting ptr to int
-    debug_printf("persistent_data is at address: %p persistent_data_memaddr_int: %ld\n", (void*)&stardata->persistent_data, persistent_data_memaddr_int);
-    
     /* free stardata (except the buffer and the persistent data) */
     binary_c_free_memory(&stardata, // Stardata
         TRUE,                       // free_preferences
@@ -1087,16 +1093,16 @@ long int return_persistent_data_memaddr(char ** const buffer,
         FALSE                       // free_persistent
     );
 
-    /* Return the memaddr as an int */
-    return persistent_data_memaddr_int;
+    /* Return the pointer */
+    debug_printf("Returning persistent_data pointer %p\n", persistent_data);
+    return persistent_data;
 }
 
 /* =================================================================== */
 /* Functions to free memory                                            */
 /* =================================================================== */
 
-// TODO: modify functon with pycapsules
-int free_persistent_data_memaddr_and_return_json_output(long int persistent_data_memaddr,
+int free_persistent_data_memaddr_and_return_json_output(struct persistent_data_t * persistent_data,
                char ** const buffer,
                char ** const error_buffer,
                size_t * const nbytes)
@@ -1105,20 +1111,7 @@ int free_persistent_data_memaddr_and_return_json_output(long int persistent_data
     struct libbinary_c_stardata_t *stardata = NULL;
     char * empty_str = "";
 
-    // persistent_data:
-    struct libbinary_c_persistent_data_t *persistent_data;
-    if(persistent_data_memaddr != -1)
-    {
-        // load the persistent data from the long int that has been passed
-        persistent_data = (void*)persistent_data_memaddr;
-        debug_printf("Took long int memaddr %ld and loaded it to %p\n", persistent_data_memaddr, (void*)&persistent_data);
-    }
-    else
-    {
-        printf("ERROR: this function needs a valid persistent_data_memaddr value. not -1\n");
-        // persistent_data = NULL;
-        // TODO: put break in the function here. 
-    }
+    debug_printf("Freeing persistent_data pointer %p\n", persistent_data);
 
     /* Set up new system */
     binary_c_new_system(&stardata,          // stardata
@@ -1129,6 +1122,7 @@ int free_persistent_data_memaddr_and_return_json_output(long int persistent_data
                         &empty_str,         // argv
                         -1                  // argc
     );
+    debug_printf("Freed persistent_data pointer.\n");
 
     /* Set the model hash (usually done internally but we're not evolving a system here */
     stardata->model.ensemble_hash = stardata->persistent_data->ensemble_hash;
@@ -1152,7 +1146,7 @@ int free_persistent_data_memaddr_and_return_json_output(long int persistent_data
         TRUE,                       // free_stardata
         TRUE,                       // free_store
         FALSE,                      // free_raw_buffer
-        FALSE                       // free_persistent. It already 
+        FALSE                        // free_persistent. It already to be sure
     );
 
     return 0;
@@ -1160,7 +1154,7 @@ int free_persistent_data_memaddr_and_return_json_output(long int persistent_data
 
 
 // TODO: modify functon with pycapsules
-int free_store_memaddr(long int store_memaddr,
+int free_store_memaddr(struct libbinary_c_store_t * store,
                char ** const buffer,
                char ** const error_buffer,
                size_t * const nbytes)
@@ -1169,19 +1163,7 @@ int free_store_memaddr(long int store_memaddr,
     struct libbinary_c_persistent_data_t *persistent_data = NULL;
     char * empty_str = "";
 
-    // Store:
-    /* Check the value of the store_memaddr */
-    struct libbinary_c_store_t *store;
-    if(store_memaddr != -1)
-    {
-        // load the store from the integer that has been passed
-        store = (void*)store_memaddr;
-        debug_printf("Took long int store_memaddr %ld and loaded it to %p\n", store_memaddr, (void*)&store);
-    }
-    else
-    {
-        store = NULL;
-    }
+    debug_printf("Freeing store pointer %p\n", store);
 
     /* Set up new system */
     binary_c_new_system(&stardata,          // stardata
@@ -1192,7 +1174,6 @@ int free_store_memaddr(long int store_memaddr,
                         &empty_str,         // argv
                         -1                  // argc
     );
-
     debug_printf("freed store memaddr\n");
 
     /* output to strings */
-- 
GitLab