Skip to content

Resolve "Assigning MultiTimestampsVector to structure doesn't properly disown memory"

Karl Wette requested to merge ANU-CGA/lalsuite:SWIG-CWMFDataParams-fixes into master

Description

Finally figured out a fix for #604 (closed): a combination of

  • a bug in SWIG's memory ownership tracking mechanism when accessing array elements
  • the multiTimestamps member of CWMFDataParams not being a MultiLIGOTimeGPSVector pointer but a struct (which is very weird, as this is not the usual idiom...) which prevents memory ownership being transferred to CWMFDataParams
  • CWMFDataParams itself not having a destructor, without which memory assigned to it isn't cleaned up

API Changes and Justification

Backwards Compatible Changes

  • This change does not modify any class/function/struct/type definitions in a public C header file or any Python class/function definitions
  • This change adds new classes/functions/structs/types to a public C header file or Python module

Backwards Incompatible Changes

  • This change modifies an existing class/function/struct/type definition in a public C header file or Python module
  • This change removes an existing class/function/struct/type from a public C header file or Python module

Making the multiTimestamps member of CWMFDataParams a struct instead of a pointer is a weird choice to begin with, and one which doesn't work well with SWIG, so needs to be changed. In existing LALPulsar code the multiTimestamps member is only ever assigned a MultiLIGOTimeGPSVector pointer (as a struct copy), so making the member itself a pointer makes C usage simpler as well.

Review Status

With this MR the example code in the description of #604 (closed) now works fine without errors/segfaults

cc @rodrigo.tenorio @david-keitel

Merge request reports