Review kdelibs whiting/fixQByteArrays

[David Faure]

On Sunday 01 May 2011, Olivier Goffart wrote:
> Le Saturday 30 April 2011, David Faure a écrit :
> > On Friday 29 April 2011, Jeremy Whiting wrote:
> > > Since David added QT_USE_FAST_OPERATOR_PLUS to our build rules for
> > > kdelibs, it wont build here.
> > 
> > Oops?
> > 
> > > 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?
> 
> Where is the patch?

In fact I was reviewing "git show origin/whiting/fixQByteArrays"
but now I see that there were earlier commits in that branch.

This looks especially ugly:
- [...] XInternAtom( disp, QByteArray( msg_type_P ) + "_BEGIN" );
+[...] XInternAtom( disp, QByteArray( QByteArray( msg_type_P ) + "_BEGIN" ) );

Ah it's because XInternAtom takes a const char*, so in theory this should even 
be QByteArray( QByteArray( msg_type_P ) + "_BEGIN" ).constData()
...

Another one:

-      envs << QString::fromLatin1( QByteArray("DISPLAY=") + dpystring );
+    envs << QString::fromLatin1( QByteArray(QByteArray("DISPLAY=") + 
dpystring) );

Is this still because QString::fromLatin1( const QByteArray& ) is missing, so 
it has to cast to const char* here too? Olivier, wouldn't it make sense to 
have such an overload? (It would save a strlen, too).

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Sponsored by Nokia to work on KDE, incl. Konqueror (http://www.konqueror.org).