Check if model_simulation is NULL before delete

[joaobrittoneto]

Set it to NULL after delete

[doudou]

delete NULL; is valid C++ (free(NULL) is NOT valid C). The only thing you must do is reset the pointer to NULL after deletion.

Why do you delete the pointer before reallocating it ? If your code is exception-safe (it should!), then this should be unneeded.

Invariants to enforce:

• if configureHook succeeds, model_simulation is non-NULL and valid. RTT ensures that cleanupHook will be called from this point on.
• if configureHook fails, model_simulation is NULL.

The best way to achieve that is to use a on-the-stack std::auto_ptr (C++98) or std::unique_ptr (C++11) to hold the pointer until it's fully initialized, and assign to model_simulation only when the method returns success with model_simulation = sim->release();

[joaobrittoneto]

Ok,
I got a segmentation fault at this point.

I'll try to use std::auto_ptr

---

[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `/home/flatfish/dev.bir/install/bin/orogen_default_uwv_dynamic_model__Task --ren'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x00000000000002f1 in ?? ()
[Current thread is 1 (Thread 0x7f729bfff700 (LWP 32087))]
(gdb) bt
#0 0x00000000000002f1 in ?? ()
#1 0x00007f72e4770e86 in uwv_dynamic_model::Task::configureHook (this=0x11afda0) at /home/flatfish/dev.bir/control/orogen/uwv_dynamic_model/tasks/Task.cpp:32
#2 0x00007f72e42e9885 in RTT::base::TaskCore::configure() () from /home/flatfish/dev.bir/install/lib/liborocos-rtt-gnulinux.so.2.8
#3 0x00007f72e472d00c in uwv_dynamic_model::TaskBase::configure (this=0x11afda0) at /home/flatfish/dev.bir/control/orogen/uwv_dynamic_model/.orogen/tasks/TaskBase.cpp:169
#4 0x00007f72e083f190 in _0RL_lcfn_10a79cd5b9c0aa8e_60000000(omniCallDescriptor*, omniServant*) () from /home/flatfish/dev.bir/install/lib/liborocos-rtt-corba-gnulinux.so.2.8
#5 0x00007f72e04ad759 in omniCallHandle::upcall(omniServant*, omniCallDescriptor&) () from /usr/lib/libomniORB4.so.1
#6 0x00007f72e084263f in RTT::corba::_impl_CTaskContext::_dispatch(omniCallHandle&) () from /home/flatfish/dev.bir/install/lib/liborocos-rtt-corba-gnulinux.so.2.8
#7 0x00007f72e04a67bd in omni::omniOrbPOA::dispatch(omniCallHandle&, omniLocalIdentity*) () from /usr/lib/libomniORB4.so.1
#8 0x00007f72e04847a8 in omniLocalIdentity::dispatch(omniCallHandle&) () from /usr/lib/libomniORB4.so.1
#9 0x00007f72e04c7bc0 in omni::GIOP_S::handleRequest() () from /usr/lib/libomniORB4.so.1
#10 0x00007f72e04c8a48 in omni::GIOP_S::dispatcher() () from /usr/lib/libomniORB4.so.1
#11 0x00007f72e04c53f5 in omni::giopWorker::real_execute() () from /usr/lib/libomniORB4.so.1
#12 0x00007f72e04c5b1f in omni::giopWorker::execute() () from /usr/lib/libomniORB4.so.1
#13 0x00007f72e047856d in omniAsyncWorkerInfo::run() () from /usr/lib/libomniORB4.so.1
#14 0x00007f72e0478bff in omniAsyncWorker::run(void*) () from /usr/lib/libomniORB4.so.1
#15 0x00007f72e01b8779 in omni_thread_wrapper () from /usr/lib/libomnithread.so.3
#16 0x00007f72e3fa56ba in start_thread (arg=0x7f729bfff700) at pthread_create.c:333
#17 0x00007f72df3113dd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
(gdb)

[joaobrittoneto]

@doudou, does it seem better?

[doudou]

Used like this, it makes no difference. If the constructor raises, then the object is never initialized. If the constructor passes, then you release it right away.

If you move the release just before the return true;, then things are becoming more interesting. If the parameters are invalid (in setUWParameters) and you raise, then you keep your invariants.

If you get a crash, run under valgrind. It should give you a better idea of what's going on.

[g-arjones]

what was going on was that he was deleting the pointer twice (cleanupHook AND configureHook)

EDIT: because he was not setting it to NULL after deleting in the cleanupHook

[doudou]

But .. the PR was adding the reinit to NULL. I'm confused. Anyways, glad I could help ;-)

[joaobrittoneto]

Thanks guy.
I tested it today and had no problem.
Merging it