Free the map target when the object is destroyed

[TSonono]

I tried to make changes to src/tools/dmlc/test/1.4/lib/T_map_target_connect.py to test this, but I couldn't find a good way to check if the map target had been free:d. Let me know if you think this is OK or if you have a good way of testing this.

[syssimics]

PR Verification: ✅ success

PR Verification FAILED

[mandolaerik]

The build success notification is broken, real failing build here: https://github.com/intel-innersource/applications.simulators.simics.simics-base/actions/runs/15248158485

(and nitpick: the "destoryed" typo remains in the commit msg)

[mandolaerik]

Regarding test, what we possibly could do would be to add an internal function int VT_map_target_refcount(map_target_t *), but doesn't need to be done within this PR.

[mandolaerik]

Regarding build errors: one thing you can do is to wrap this in a group _internal_destructor is destroy to avoid clashes.

[lwaern-intel]

That is a solution, but is it desirable to always and unconditionally free the map target?

[TSonono]

Regarding build errors: one thing you can do is to wrap this in a group _internal_destructor is destroy to avoid clashes.

Would that be preferred? I could fix these particular build errors in pcie-downstream-port and cxl-downstream-port in the base repo.

[TSonono]

That is a solution, but is it desirable to always and unconditionally free the map target?

If the object is deleted, is there any scenario where not freeing the map target makes sense?

[lwaern-intel]

Yes, I was about to say. The build issues are due to various sporadic destroy_tmpl templates scattered across the repo that was a workaround before the destroy template was officially added. These could be addressed by simply cleaning up the old usages of destroy_tmpl and using destroy instead.

[lwaern-intel]

That is a solution, but is it desirable to always and unconditionally free the map target?

If the object is deleted, is there any scenario where not freeing the map target makes sense?

Dunno. I figure not, save, peraps, some extremely bad exotic interactions, which would constitute separate bugs in their own right.

[lwaern-intel]

Or well -- perhaps there'd be merit in making it an overridable method of the connect, because that'd allow the user to act on the connected object before freeing it (by calling default()). An internal destructor would make that impossible. But I don't know how realistic such a use case would be.

In general, the question of overrides are something to consider. If there is a destroy() specified, and the user doesn't call default() (whether they be because the code precedes this PR or because they forgot), then the map target won't get freed. Having an internal destructor object prevents that situation from occurring. Another consideration is that destroy of subobjects are guaranteed to be called first before their parent objects. This means there is no issue if there's a custom destroy of the connect that also frees the map target -- because this internal destructor will set the map_target to NULL, and SIM_free_map_target is a noop on NULL.

So there are pros and cons here. I'm leaning towards the internal destructor, though.