Github user arpadboda commented on a diff in the pull request: https://github.com/apache/nifi-minifi-cpp/pull/432#discussion_r231884707 --- Diff: nanofi/src/api/nanofi.cpp --- @@ -323,55 +360,45 @@ int transmit_flowfile(flow_file_record *ff, nifi_instance *instance) { flow * create_new_flow(nifi_instance * instance) { auto minifi_instance_ref = static_cast<minifi::Instance*>(instance->instance_ptr); - flow *new_flow = (flow*) malloc(sizeof(flow)); - - auto execution_plan = new ExecutionPlan(minifi_instance_ref->getContentRepository(), minifi_instance_ref->getNoOpRepository(), minifi_instance_ref->getNoOpRepository()); - - new_flow->plan = execution_plan; - - return new_flow; + return new flow(minifi_instance_ref->getContentRepository(), minifi_instance_ref->getNoOpRepository(), minifi_instance_ref->getNoOpRepository()); --- End diff -- Multiple thins to add there: - Please note that this is not an introduced "new" call, just the one 5 lines above. - We can change this to use malloc and placement new, in which case it ensures that free only causes some leak, but no crash. - I don't think this approach is sustainable in long term. To properly free structures the users would be required to know the internal layout of the structure, which is not going to happen and it would lead to very complex free strategies in case of shared ownerships. Moreover I don't think that the approach "feel free to free, it's just a leak" is the way forward. One of the reasons I only left declarations in cstructs.h is that freeing something you don't even know should make you raise concerns. What I would really push is the user freeing up all the stuff he allocated and we provide free functions for every structure that the API may return. Don't free what you don't allocate is quite clear and well defines responsibilities. To go even further here, letting the users free things might be a painful barrier in case we plan to change the internal representation, which they shouldn't be aware of anyway. In nutshell, combining free with placement new is good enough to avoid immediate crashes, but we should try to highlight that the structures allocated by the API should also be freed by that.
---