Re: Cppcheck: false positive in pptx-text.cxx ?

Hey,

2013/1/22 Julien Nabet <serval2412@yahoo.fr>:
> On 22/01/2013 16:37, Markus Mohrhard wrote:
> > > I'll do this one. You can see then how it works. You should maybe read
> > > http://www.boost.org/doc/libs/1_52_0/libs/smart_ptr/shared_ptr.htm or
> > > some other resource about shared_ptr.
> > See
> > http://cgit.freedesktop.org/libreoffice/core/commit/?id=ec8ff9ff68622a510319c404f4745a336e65d314
> > for an example. Please note that I did some additional clean-up. The
> > important parts are that in this case operator= and the copy c'tor are
> > no longer needed.
> >
> > The remaining parts are mainly stylistic and moving the Impl class
> > from the header file into the source file.
> >
> > If you try to change additional places please inspect them very
> > carefully. As I and Eike already mentioned, shared_ptr comes with a
> > performance penalty that may or may not be a problem.
>
> To be clear, I thought that ref count had been put in place long time ago
> when there wasn't efficient libraries/version of libraries. So when you
> first answered by quoting boost::shared_ptr, I thought it could be a 100%
> better solution than ref count.

boost::scoped_ptr is just a reference counted pointer container.

> Now with this performance penalty, I don't know what to think:
> - don't touch anything and so keep ref counting
> - find a specific solution for each case using than ref counting
> - use boost::shared_ptr (but for which case which wouldn't generate perf
> penalty?)
>
> I must recognize, I had naively thought that ref count was a part where we
> could make some bug proof/optimization/cleaning LO

Ref counting is a valid pattern for memory management. Whether this
happens as part of a container class or in some other way is just an
implementation detail.

However as mentioned there are cases like your original report where
replacing a manual reference counting implementation can be safely
replaced with a generic solution. In this case it helped to clean the
code a bit.

IMHO each case where we are using reference counting needs to be
treated separately and you'd need to understand the implications of
using boost::shared_ptr in that case. For example inside calc we try
to avoid boost::shared_ptr for performance reasons.

After a quick look through all places where mnRefCount is mentioned
the only file that looks quite safe for a replacement is
http://opengrok.libreoffice.org/xref/core/filter/source/msfilter/svdfppt.cxx
Especially the ones in vcl and drawinglayer may need a very close look
before one can decide if it makes sense to replace any reference
counting in them.

Regards,
Markus