Dangerous constructor in Handle class

[imachabeli]

What is the purpose of this constructor for the handle class?

template <class T>
inline Handle<T>::Handle(T* p,bool registerAsObserver)
: link_(new Link(boost::shared_ptr<T>(p),registerAsObserver)) {}

If you create handle to something that you plan to delete later   or
even worse to something that is on the stack (like I did by accident)
implicit boost pointer  passed to Link will try to delete object that is not
supposed to be deleted.
In my case luckily CRT caught invalid heap address.

Anyway, I commented out this constructor and both Quantlib and test suite
compile and run. The only place this constructor is used us in
fdmbatesop.cpp (under experimental\finitedifference) and existing code can
be easily replaced with the following
   FdmBatesOp::FdmBatesOp(
                    const boost::shared_ptr<FdmMesher>& mesher,
                    const boost::shared_ptr<BatesProcess>& batesProcess,
                    const FdmBoundaryConditionSet& bcSet,
                    const Size integroIntegrationOrder,
                    const boost::shared_ptr<FdmQuantoHelper>& quantoHelper)
    : lambda_(batesProcess->lambda()),
      delta_ (batesProcess->delta()),
      nu_    (batesProcess->nu()),
      m_(std::exp(nu_+0.5*delta_*delta_)-1.0),
      gaussHermiteIntegration_(integroIntegrationOrder),
      mesher_(mesher),
      bcSet_(bcSet),
      hestonOp_(new FdmHestonOp(mesher,
           boost::shared_ptr<HestonProcess>(new HestonProcess(
               batesProcess->riskFreeRate(),
               Handle<YieldTermStructure>(
                    boost::shared_ptr<ZeroSpreadedTermStructure>(
                   new ZeroSpreadedTermStructure(
                           batesProcess->dividendYield(),
                           Handle<Quote>(boost::shared_ptr<Quote>(new
SimpleQuote(lambda_*m_))),
                           Continuous, NoFrequency,
                           batesProcess->dividendYield()->dayCounter()))),
               batesProcess->s0(),    batesProcess->v0(),
               batesProcess->kappa(), batesProcess->theta(),
               batesProcess->sigma(), batesProcess->rho())),
           quantoHelper)) {
    }

I just wrapped new in couple places inside a shared pointers


------------------------------------------------------------------------------
Magic Quadrant for Content-Aware Data Loss Prevention
Research study explores the data loss prevention market. Includes in-depth
analysis on the changes within the DLP market, and the criteria used to
evaluate the strengths and weaknesses of these DLP solutions.
http://www.accelacomm.com/jaw/sfnl/114/51385063/
_______________________________________________
QuantLib-dev mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quantlib-dev

[Luigi Ballabio]

On Tue, 2011-07-19 at 18:03 -0400, Irakli Machabeli wrote:
> What is the purpose of this constructor for the handle class?
>
> template <class T>
> inline Handle<T>::Handle(T* p,bool registerAsObserver)
> : link_(new Link(boost::shared_ptr<T>(p),registerAsObserver)) {}

I guess it's just a shortcut to avoid declaring the boost shared pointer
explicitly.  It's not much more dangerous than the shared_ptr
constructor (you can have the same problem) but I agree that at least it
should be documented.  I wouldn't remove it, because we're keeping
backward compatibility with previous releases; but we can mark it as
deprecated.

Luigi

>
> If you create handle to something that you plan to delete later   or
> even worse to something that is on the stack (like I did by accident)
> implicit boost pointer  passed to Link will try to delete object that is not
> supposed to be deleted.
> In my case luckily CRT caught invalid heap address.
>
> Anyway, I commented out this constructor and both Quantlib and test suite
> compile and run. The only place this constructor is used us in
> fdmbatesop.cpp (under experimental\finitedifference) and existing code can
> be easily replaced with the following
>    FdmBatesOp::FdmBatesOp(
>                     const boost::shared_ptr<FdmMesher>& mesher,
>                     const boost::shared_ptr<BatesProcess>& batesProcess,
>                     const FdmBoundaryConditionSet& bcSet,
>                     const Size integroIntegrationOrder,
>                     const boost::shared_ptr<FdmQuantoHelper>& quantoHelper)
>     : lambda_(batesProcess->lambda()),
>       delta_ (batesProcess->delta()),
>       nu_    (batesProcess->nu()),
>       m_(std::exp(nu_+0.5*delta_*delta_)-1.0),
>       gaussHermiteIntegration_(integroIntegrationOrder),
>       mesher_(mesher),
>       bcSet_(bcSet),
>       hestonOp_(new FdmHestonOp(mesher,
>            boost::shared_ptr<HestonProcess>(new HestonProcess(
>                batesProcess->riskFreeRate(),
>                Handle<YieldTermStructure>(
>                     boost::shared_ptr<ZeroSpreadedTermStructure>(
>                    new ZeroSpreadedTermStructure(
>                            batesProcess->dividendYield(),
>                            Handle<Quote>(boost::shared_ptr<Quote>(new
> SimpleQuote(lambda_*m_))),
>                            Continuous, NoFrequency,
>                            batesProcess->dividendYield()->dayCounter()))),
>                batesProcess->s0(),    batesProcess->v0(),
>                batesProcess->kappa(), batesProcess->theta(),
>                batesProcess->sigma(), batesProcess->rho())),
>            quantoHelper)) {
>     }
>
> I just wrapped new in couple places inside a shared pointers
>
>
> ------------------------------------------------------------------------------
> Magic Quadrant for Content-Aware Data Loss Prevention
> Research study explores the data loss prevention market. Includes in-depth
> analysis on the changes within the DLP market, and the criteria used to
> evaluate the strengths and weaknesses of these DLP solutions.
> http://www.accelacomm.com/jaw/sfnl/114/51385063/
> _______________________________________________
> QuantLib-dev mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/quantlib-dev

--

Just remember what ol' Jack Burton does when the earth quakes, the
poison arrows fall from the sky, and the pillars of Heaven shake. Yeah,
Jack Burton just looks that big old storm right in the eye and says,
"Give me your best shot. I can take it."
-- Jack Burton, "Big trouble in Little China"



------------------------------------------------------------------------------
10 Tips for Better Web Security
Learn 10 ways to better secure your business today. Topics covered include:
Web security, SSL, hacker attacks & Denial of Service (DoS), private keys,
security Microsoft Exchange, secure Instant Messaging, and much more.
http://www.accelacomm.com/jaw/sfnl/114/51426210/
_______________________________________________
QuantLib-dev mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quantlib-dev

[Ferdinando M. Ametrano-3]

On Wed, Jul 20, 2011 at 11:05 AM, Luigi Ballabio
<[hidden email]> wrote:

> On Tue, 2011-07-19 at 18:03 -0400, Irakli Machabeli wrote:
>> What is the purpose of this constructor for the handle class?
>>
>> template <class T>
>> inline Handle<T>::Handle(T* p,bool registerAsObserver)
>> : link_(new Link(boost::shared_ptr<T>(p),registerAsObserver)) {}
>
> I guess it's just a shortcut to avoid declaring the boost shared pointer
> explicitly.  It's not much more dangerous than the shared_ptr
> constructor (you can have the same problem) but I agree that at least it
> should be documented.  I wouldn't remove it, because we're keeping
> backward compatibility with previous releases; but we can mark it as
> deprecated.

done

ciao -- Nando

------------------------------------------------------------------------------
Got Input?   Slashdot Needs You.
Take our quick survey online.  Come on, we don't ask for help often.
Plus, you'll get a chance to win $100 to spend on ThinkGeek.
http://p.sf.net/sfu/slashdot-survey
_______________________________________________
QuantLib-dev mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quantlib-dev