"-DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_TO_ASCII" as policy in kde

Parker Coates parker.coates at kdemail.net
Mon Nov 15 23:04:40 GMT 2010


On Mon, Nov 15, 2010 at 13:25, David Faure wrote:
> On Wednesday 06 October 2010, Parker Coates wrote:
>>  But I just don't think wrapping every
>> single literal with a constructor or conversion function is improving
>> code quality. [2]
>
> Right. In the code you linked to, it's apparent that
> 1) KIcon is almost always used with a const char* litteral as argument, so we
> should add a set of const char* constructor overloads to KIcon
> (IIRC this means we have to add qbytearrays too, otherwise existing code using
> a qbytearray becomes ambiguous... This sounds like an unlikely case though, so
> I don't know if we need to do that, maybe I can check compilation of
> trunk/KDE+extragear without qbytearray constructors and ignore the corner case
> to avoid unnecessary API bloat?)
>
> 2) KActionCollection::addAction() is also almost always used with "..."
> litterals, so maybe we should do the same there (but then this could be
> applied to every API that uses action names, and there might be much of it).

That is an interesting compromise, but one would have to decide how
far to take it. I just ran through all of Laurent's QT_NO_CAST_*
commits in KDEGames and the following methods showed up repeatedly as
being called with literal strings. The number of asterisks indicates
approximately how often each showed up.

** QObject::setObjectName()
* QSvgRenderer::boundsOnElement()
* QSvgRenderer::render()

** KActionCollection::action()
*** KActionCollection::addAction()
** KConfigDialog::KConfigDialog()
** KConfigDialog::addPage()
** KConfigDilaog::showDialog()
* KDialog::setHelp()
*** KIcon::KIcon()
*** KLocale::insertCatalog()
** KMessageBox, almost every static method's dontAskAgain parameter
* KNotification::event()
** KStandardDirs::locate()
* KStandardDirs::locateLocal()
** KStandardDirs::findAllResources()
* KXmlGuiClient::plugActionList()

** KGameRenderer::spritePixmap()
** KGameRendererClient::KGameRendererClient() (and children)
* KGameRendererClient::setSpriteKey()
** KScoreDialog::addField()
* KScoreDialog::setConfigGroup()

> PS: after this thread, I made kdecore fully compile with
> QT_NO_CAST_FROM_ASCII. Found 3 encoding bugs. But indeed let's do this right
> if we do it above in the stack too. Icon names and action names are a very
> very unlikely place for encoding bugs...

How many of these are likely to be subject to encoding bugs? In the
majority of the above cases, QStrings are being used to represent
unique identifiers for things. In such cases, literal use is very
common. Others, like KStandardDirs, show up because they are often
called with a constant string, but the string here actually represents
a string of characters and not an identifier, so these methods
probably don't need a "const char *" alternative.

Regardless of the above, I think that if a new policy is going to be
created for all KDE SC *applications*, kde-core-devel probably isn't
the place to make that decision.

Parker




More information about the kde-core-devel mailing list