Angus Leeming <[EMAIL PROTECTED]> writes:
| Lars Gullik Bj�nnes wrote:
>
>> Since we are almost tempted to use exceptions...
>>
>> This is aimed at fixing memory leaks in the case of exceptions...
>> (probably not all good, neither does it fix all problems...)
>
| Given that renderer_ is stored as a scoped_ptr<RenderBase> already,
| this code seems ugly, ugly, ugly. Perhaps it would be better to store
| the thing in a shared_ptr<RenderBase>?
Not sure what you find particulary ugly, the release()? I think my
changes below makes the code a bit nicer.
| Question: given that RenderGraphic derives from RenderBase, would code
| like this work:
| boost::shared_ptr<RenderGraphic> graphic_ptr(new RenderGraphic);
| renderer_ = graphic_ptr;
| graphic_ptr->connect(boost::bind(&InsetExternal::statusChanged,
| this));
>
| Ie, does 'shared_ptr<BaseClass> = shared_ptr<DerivedClass>' work?
yes, as would shared_ptr<BaseClass> = auto_ptr<DerivedClass>.
| @@ -580,7 +580,8 @@ void InsetExternal::setParams(InsetExter
| case RENDERBUTTON: {
| RenderButton * button_ptr = renderer_->asButton();
| if (!button_ptr) {
| - button_ptr = new RenderButton;
| + auto_ptr<RenderButton> rb(new RenderButton);
| + button_ptr = rb.release();
| renderer_.reset(button_ptr);
One of the places I was a bit eager it seems.
renderer_.reset(new RenderButton);
button_ptr = renderer_->asButton();
would perhaps be nicest.
| @@ -590,7 +591,8 @@ void InsetExternal::setParams(InsetExter
| } case RENDERGRAPHIC: {
| RenderGraphic * graphic_ptr = renderer_->asGraphic();
| if (!graphic_ptr) {
| - graphic_ptr = new RenderGraphic;
| + auto_ptr<RenderGraphic> rg(new RenderGraphic);
| + graphic_ptr = rg.release();
| graphic_ptr->connect(
| boost::bind(&InsetExternal::statusChanged, this));
| renderer_.reset(graphic_ptr);
I'd keep this but drop the "graphic_ptr = rg.release()" line so that:
auto_ptr<RenderGraphic> rg(new RenderGraphic);
rg->connect(boost::bind(&Insetexternal::statusChanged, this));
renderer_.reset(rg);
graphic_ptr = renderer_->asGraphic();
is the function instead.
| @@ -604,7 +606,9 @@ void InsetExternal::setParams(InsetExter
| RenderMonitoredPreview * preview_ptr =
| renderer_->asMonitoredPreview();
| if (!preview_ptr) {
| - preview_ptr = new RenderMonitoredPreview;
| + auto_ptr<RenderMonitoredPreview>
| + rmp(new RenderMonitoredPreview);
| + preview_ptr = rmp.release();
| preview_ptr->connect(
| boost::bind(&InsetExternal::statusChanged, this));
| preview_ptr->fileChanged(
similar here.
auto_ptr<RenderMonitoredPreview>
rmp(new RenderMonitoredPreview);
rmp->connect(boost::bind(&InsetExternal::statusChanged, this));
rmp->fileChanged(...
renderer_.reset(rmp);
preview_ptr = renderer_->asMonitoredPreview();
I do not find these ugly then...
--
Lgb
