Review kdelibs whiting/fixQByteArrays
jpwhiting at kde.org
Sat Apr 30 03:10:50 BST 2011
On Apr 29, 2011 6:26 PM, "David Faure" <faure at kde.org> wrote:
> On Friday 29 April 2011, Jeremy Whiting wrote:
> > Since David added QT_USE_FAST_OPERATOR_PLUS to our build rules for
> > it wont build here.
> > I've got a commit on the whiting/fixQByteArrays that
> > fixes this, but I'd like someone to review it before I merge to master.
> > Anyone got a moment to test and review it?
> Reviewboard is a better tool for this, but okay, I'm not in a position to
> complain, as the guy with the hat of shame for breaking the build :)
It could be because arch recently switched to gcc 4.6. Could also be because
I'm trying to use qt 4.8 from git. I'll take a try at your suggestions when
I get up in the morning.
> - QString testpath = "file://" + path + "/kdelibs/khtml/test/";
> + QString testpath = QString("file://" + path +
> Can be done simpler as
> QString testpath("file://" + path + "/kdelibs/khtml/test/");
> Surprisingly (or rather unsurprisingly, since I did test before
> it compiles without this change. gcc-4.4.5 here, kubuntu; which compiler
> you use?
> This is a case of an explicit conversion to QString so in theory it should
> just work (unlike all the template cases I fixed in my commit), but maybe
> breaks on some compilers because the first arg is a char litteral.
> I guess a more logical change, then, could be
> QString testpath = QString("file://") + path + "...";
> The other changes are related to QByteArray, which leaves me completely
> stumped. Fast operator plus is for QString only. How can m_tokens[id+uuid]
> break, when id and uuid are bytearrays? Oh, m_tokens takes a QString as
> Please test and apply the real fix:
> - QMap<QString, QByteArray> m_tokens;
> + QMap<QByteArray, QByteArray> m_tokens;
> AFAICS the key is used as a bytearray everywhere in
> For the last change:
> - removeDir(QLatin1String("packageRoot/" + dir.toLatin1() +
> + removeDir(QLatin1String(QByteArray("packageRoot/" +
> I think it would be simpler to write
> removeDir(QLatin1String("packageRoot/") + dir + "/contents/code"));
> (or QString::fromLatin1 instead of QLatin1String, if necessary)
> (and maybe with a QLatin1String around the last arg, if this code uses "no
> cast from ascii". The compiler will tell you ;) ).
> David Faure, faure at kde.org, http://www.davidfaure.fr
> Sponsored by Nokia to work on KDE, incl. Konqueror (
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the kde-core-devel