Using smart pointers with implicit conversion to raw pointer types is dangerous. Reference: Modern C++ Design: Generic Programming and Design Patterns Applied, 7.7 Implicit Conversion to Raw Pointer Types (http://www.informit.com/articles/article.aspx?p=31529&seqNum=7) Remove implicit conversion to raw pointer in SvRef (SvRef::operator T*()). Fix errors using get() method (SvRef::get())
Jan, could you look? Thanks
I could, but this is one of those, where I rely on the opinion of our C++ gurus. Sounds logical to me, and if Stephan agrees, it should be made a easyhack. I am putting it on NEEDINFO for now (We also miss a ref. to where SvRef lives).
Yes, looks reasonable. Ultimately, uses of SvRef (include/tools/ref.hxx) should be replaced with other smart pointers (the standard ones, or maybe LO's rtl::Reference), but for the time beeing this is probably a good clean up.
Hello. Can i try to assign this bug to me? It will be my first bug. And i hope, that i can solve it.
If i correctly understood task, i should: 1) Remove all using SvRef::operator*() in LO sourcebase and replace to SvRef::get() 2) Remove from SvRef operator*() or mark it as deleted of simply remove this operator from SvRef.
The problematic operator is the (non-explicit) conversion operator > operator T *() const { return pObj; } not the dereference operator > T & operator *() const { assert(pObj != nullptr); return *pObj; } All uses of the former operator, and the operator itself, shall be removed. (There is no benefit in keeping it around defined as deleted. But it can be useful to temporarily define it as deleted, to find places that use this conversion operator.) Replacing uses of the latter operator with get() is of no benefit. (But removing the address-of operator > T * operator &() const { return pObj; } would be beneficial, as an overloaded address-of operator can be confusing.)
Hi, since this is not assinged to anyone I have started working on it. Can removal of > T * operator &() const { return pObj; } also be included here?
(In reply to Jacek Frączek from comment #7) > Hi, since this is not assinged to anyone I have started working on it. Can > removal of > > T * operator &() const { return pObj; } > also be included here? That's a reasonable change too, but better do it in two different commits (that can both be part of this easy hack).
Jacek Fraczek committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=f004aa99514d385f3ee254bba735f5eaeb7d9ad8 tdf#89307: Removed SvRef::operator T*() It will be available in 5.3.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Jacek Fraczek committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=398d641664baa6eaeb34789f0aebfd21e73edef3 tdf#89307: Removed T* SvRef::opeartor &() It will be available in 5.3.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.