Added free mask where necessary to fix memory leak. #45

[Phelimb]

@iqbal-lab this is just the first of these memory leaks but just wanted to get your opinion on this solution. Is this what you would have suggested doing? Simply freeing mask when finished. It seems a little confusing to me when the free isn't even in the same file as the malloc.

Is the alternative is to alloc the masks within the correct scope and then use void functions to update them? Is this the recommended method do you know?

[iqbal-lab]

This is the whole issue of having a clear policy of ownership. It is indeed confusing and a source of bugs to free and malloc in different files. If we were writing in C++, you'd have an object which contained a constructor for malloc and a destructor for free, but you would still have the issue of knowing whose job it is to destroy an object. In our case the system is very simple - we don't have complex objects being created all over the place at runtime and being passed back and forth. Hold on - I'll take a look at your proposed change

[iqbal-lab]

This looks fine.

boolean non_aureus_panels_are_present(SpeciesInfo* species_info){ boolean* mask = create_non_aureus_mask(); boolean non_aureus_species_panels_are_present = panels_are_present(species_info->species_covg_info,mask);

• // Finished with the mask so we need to free it.
• free(mask); return (non_aureus_species_panels_are_present); }

One simple and effective thing we could do is have functions which alloc and return an object, ownership of which is transferred to the caller, have a certain word in the function-name. eg create, in fact! Anyway - this chunk, where you alloc, use, and free, seems fine. One sec, going back to code.

[iqbal-lab]

I think this looks fine.

"Is the alternative is to alloc the masks within the correct scope and then use void functions to update them? Is this the recommended method do you know?"

the alternatives are

1. don't use the heap at all, but create a mask on the stack, and then pass a pointer to it into panels_are_present(). This would work, but I expect someday/time you'll expand the panel and end up blowing the stack.
   1. alloc it on the heap in the calling function, then pass a pointer to it into panels_are_present(), and then free it.

I think your solution is fine. Have you checked it with valgrind?

[iqbal-lab]

Happy to merge this once valgrind checked

[iqbal-lab]

Spoke to Phelim - he has checked with valgrind. Fixes the leaks it is intended to fix. Still more to do.