Clean up memory management in IrSignal and IrSequence

[bengtmartensson]

Probably the IrSignal::intro/repeat/ending etc should be referenced, or pointed-to instead of copied. Dispose stategy unclear, cf. IrSignal::~IrSignal.

Cf. #14 & #15. The latter makes Pronto::mkSequence slightly unpleasant: generates a dynamic object that neither will be completely disposed by its destructor, nor does it consist of "borrowed" data (that is what toBeFreed = false is intended for).

[mjbcopland]

Point taken in regards to your comment on #15. The change fixes the mentioned memory leak but changes the behaviour of mkSequence and may introduce leaks elsewhere.

I'll take a crack at rewriting using references and move semantics over the weekend.

[mjbcopland]

I've made a few updates which correct the behaviour of mkSequence, The key change is that if we initialise an IrSequence from another IrSequence with toBeFreed=true and orig.toBeFreed=true, orig.toBeFreed is set to false.

This means that mkSequence can initialise toBeFreed as true, but then when the sequences are copied by an IrSignal the property can be set to false, allowing deletion of the original sequences without affecting the IrSignal.

These changes can be seen in #18.

A more modern approach would be to completely remove the toBeFreed properly and instead use move semantics with lvalue and rvalue references. An example of this can be seen in my mem-mgmt branch. The gist of it is that wherever toBeFreed was false we now use an lvalue reference, and where toBeFreed was true we use an rvalue reference.

Ideally, we would also remove returning pointers to dynamically allocated objects in favour of the object itself, again with move semantics to make this efficient. This would drastically reduce the memory management headache, but likely requires reworking a substantial portion of the library. I'd like to keep working on this idea on the mem-mgmt branch, but can't promise anything.

[bengtmartensson]

Quick, non-final feedback:

Thank you very much for the effort.

There is no need for an isolated fix for the "non-standard semantics" of mkSequence. The reason is that it is a private function, called only at one place; and that usage I have checked: Therefore, it is not a problem that needs to be fixed. (If it was, I would not have accepted the PR :-).)

However, I am interested in getting it "really" right; your branch and the move-semantics look very interesting.

[bengtmartensson]

The toBeFreed-stuff is awful, and should be disposed. One way may be to make IrSequence (IrSignal) abstract and implement it in two different classes, one owning the data (like toBeFeed == true), one not (toBeFreed == false).

And, as previously said, the move semantics should be implemented.

[bengtmartensson]

Also, get rid of the variable lengths arrays (Pronto.cpp line 39), since it is not a part of the official C++ standard.