MiguelMValero / CONES

CONES: This git repository aims to couple the CFD software OpenFOAM with any other kind of open-source codes. It is currently employed to carry out sequential Data Assimilation techniques and, more specifically, an Ensemble Kalman Filter (EnKF). The communications between the EnKF and OpenFOAM are performed by a coupler called CWIPI.
6 stars 1 forks source link

Better Safe Than Segfaults: passing arrays to void functions #3

Closed PaoloErrante closed 3 months ago

PaoloErrante commented 1 year ago

Hello guys,

I am enjoying this saturday taking a look to the code. I have few tips to give:

1) I have noticed that there are many void functions taking pointers as arguments and I think you are using these functions to manipulate global arrays.

void functions are great, but come with some dangers: there is not a clear distinction in between what is the input and what is the output of the function.

Normally we do not want to functions to modify inputs, so it is nice to put a "const" before and order inputs and outputs in order like in the following way:

void function(const double* input, double* output) 2) There is a lot of "mallocs" around to allocate dynamic memory for arrays. 2.1) calloc is a function that does the same thing, and prevents you to do that annoying multiplication so instead of: double* array = malloc(size * sizeof(double)); better use: double* array = calloc(size, sizeof(double)); Another good thing that calloc does is that it initializes the memory allocated.

2.2) Always check that memory allocation is done and stop the program if it is not the case: if(array* == NULL) { printf("\n No memory has been allocated for array."); exit(1); } 2.3) If there is a malloc there is a free: free arrays as you have finished with them. Especially in a coupling context like this one, the risk of memory leakage is really high at each exchange.

I am sure you are already following these simple things. I got distracted easily so I try to save a bit of my mental health with these tricks. It is worth to review the code and check this is applied before committing.

Have a nice weekend

Paolo

MiguelMValero commented 1 year ago

Hello Paolo,

Thanks for the suggestions. About the points you said, there are some aspects we are already implementing, but it is interesting the discussion you launched. During the week, I will look at how we are building the "void" functions, using "calloc" functions, and implementing conditional statements to verify that memory allocation has been performed.

Kind regards, Miguel