Ticket #28 (closed task: fixed)

Opened 15 years ago

Last modified 14 years ago

Memory management

Reported by: smidl Owned by: smidl
Priority: major Milestone: Design issues
Component: bdm core Version:
Keywords: Cc: smidl@…, miroslav.pistek@…

Description (last modified by smidl) (diff)

At present, we have very slack policy for memory management.

The only "rule" is that all offsprings of class root must have a constructor with no parameters. The actual construction is done by methods: from_setting or set_parameters.

This gets complicated when pointers are used, e.g. in class merger where method set_sources obtains pointers from outside, and these pointers may or may not be used outside of it. At present it is solved by variable "bool own" in merger. In other classes (e.g. PF or MPF) it is not solved yet.

We need:

  1. to define a policy on this - an agreed way how to handle these situations.
  2. the policy should be written as a documentation file - preferably library/doc/local/memory_management.dox
  3. the code is to be checked if the policy is consistently implemented

Change History

  Changed 15 years ago by smidl

  • description modified (diff)

  Changed 15 years ago by smidl

Another issue - I am not sure if related - is handling of arguments returned by pointer.

E.g. virtual epdf* epdf::marginal(...)

At present it is assumed that the caller of the function takes over the ownership of the pointer.

However, I can think of situations when the value is "cached" inside of the original object or even updated in time.

  Changed 15 years ago by vbarta

I think that basically, every function which takes and/or returns a pointer should have documentation specifying whether that pointer can be null and who owns the pointed-to object. That's obviously a pain to document, but from the QA perspective, interfaces with pointers _should_ be discouraged - it's much simpler and safer to use smart pointers instead (although it can be less transparent and less efficient). There are no smart pointers in BDM yet (not counting the standard auto_ptr), but I'm working on adding one (or more) - if you have specific places you'd like to port to use smart pointers, let me know.

follow-up: ↓ 5   Changed 15 years ago by vbarta

On the subject of constructors, I'd like every constructor to initialize each field of the constructed object (which doesn't always happen now) - it may well not cause any _actual_ problems (because there's no path accessing the uninitialized field), but it's much easier to think about objects when they're in defined, internally consistent state - thoughts?

in reply to: ↑ 4   Changed 15 years ago by smidl

Replying to vbarta:

On the subject of constructors, I'd like every constructor to initialize each field of the constructed object (which doesn't always happen now) - it may well not cause any _actual_ problems (because there's no path accessing the uninitialized field), but it's much easier to think about objects when they're in defined, internally consistent state - thoughts?

I am very much in favor of constructing consistent objects - when possible. Uninitialized attributes are bugs in that case. However, since we require existence of empty constructors, there is sometimes no way to check consistency (could be possibly done in the new method validate()). For example, EKF and other objects taking pointers as arguments of constructors of set_parameters().

  Changed 15 years ago by smidl

  • cc smidl@…, vbar@… added

  Changed 14 years ago by smidl

  • cc miroslav.pistek@… added; vbar@… removed

Systematically add validate() function then, and enforce its use in the whole library.

  Changed 14 years ago by smidl

  • status changed from new to closed
  • resolution set to fixed

Closing, the only remaining thing is solved in ticket #40

Note: See TracTickets for help on using tickets.