EBN const-ref fixes
David Faure
faure at kde.org
Sat Apr 28 18:17:04 BST 2007
On Saturday 28 April 2007, Harri Porten wrote:
> On Thu, 26 Apr 2007, Alex Merry wrote:
>
> > Are these const-ref return fixes in kdelibs/kdecore OK to commit (on
> > Monday, of course)?
>
> Have you doubled-check potential side effects of this one (and the 2nd
> occurrence of the same statement):
>
> --- config/ksharedconfig.cpp (revision 658327)
> +++ config/ksharedconfig.cpp (working copy)
> @@ -78,7 +78,7 @@ KSharedConfigPtr::~KSharedConfigPtr()
> delete d;
> } else if (d->ref == 1 && d->componentData().isValid()) {
> // it might be KComponentData holding the last ref
> -
> const_cast<KComponentData&>(d->componentData())._checkConfig();
> + d->componentData()._checkConfig();
>
> The _checkConfig() might now happen on a temporary copy rather than on the
> original instance. Depends on the semantics of the class and function,
> though.
Well spotted. The refcounting issues between KSharedConfigPtr and KComponentData
are complex, and this change breaks the code. Note how the code actually checks
the value of the refcount, and it does inside KComponentDataPrivate::checkConfig too,
so doing it on a copy will break since it indicate a higher refcount than expected.
And this is really a case where returning a const ref is ok imho, KComponentData
will always hold a KSharedConfig::Ptr member and KConfigBase will always hold
a KComponentData member (well it should move to KConfig but that's orthogonal).
I suggest leaving KComponentData and KConfig alone, but the rest of the changes are fine.
--
David Faure, faure at kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).
More information about the kde-core-devel
mailing list