Cppcheck: false positive in pptx-text.cxx ?

Markus Mohrhard markus.mohrhard at googlemail.com
Mon Jan 21 14:01:21 PST 2013


Hey Julien,

2013/1/21 julien2412 <serval2412 at yahoo.fr>:
> Thank you for your feedback about this Markus.
> The problem is, I don't know how to do this, would you have an example? I
> read again about copy constructor, copy-swap idiom, C++11  standard, etc. so
> a conversion example to follow could be very useful. I tried to find one in
> cgit history but didn't find it (or missed it?)

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.

>
> Moreover, I made a search about mnRefCount in OpenGrok, it seems manual ref
> has been used more than once. I don't remember in which bug I read this but
> it could be a source of problem (I think about crash problems)

As long as it is implemented correctly it is not a problem. All these
cases have to be inspected carefully and it has to be checked why
manual reference counting has been implemented and they did not use a
shared_ptr. Please be careful when you cahnge manual ref counted
classes to a shared_ptr. We have internally several own shared_ptr
classes or replacements for shared_ptr that are doing ref counting.
Additionaly boost::shared_ptr is no silver bullet to all problems and
comes with some own problems like a performance penalty.

>
> So from a conversion example given, I'd be interested to apply the
> replacement of the refCount by boost::shared_ptr in some (all if possible)
> other places.

Please remember that some of our low level classes are manually ref
counted and should not be changed or might only be changed with a lot
of more work.

>
> What do you think? Would it be useful ? Can the conversion mechanism be
> "quite automatically" be applied or each case is "unique"?

I would not change them with a careful inspection. I checked this
special case you originally mentioned before I wrote my mail so I was
sure that it is safe to use boost::shared_ptr there.

>
> The problem isn't just the cppcheck report (even if I appreciate to kill
> them :-)), I just wonder if it can help to reduce the number of crash cases.
>

I checked the code and there is no error in the handling of this code
that would be the reason for a crash. That does not mean that it is
not a good idea to switch to a shared_ptr, as this means less code and
less chance for future bugs.

Regards,
Markus


More information about the LibreOffice mailing list