Skip to content
Permalink
Browse files
Flip a couple of Q_FOREACHs to c++11 for loops
... just to check how bad the Q_FOREACH deprecation will be. And yep,
it's horrendous. Each one takes around 10 seconds or so to port, and
we've got some 2500+ remaining uses.
  • Loading branch information
nyalldawson committed Aug 29, 2017
1 parent 0cb52f6 commit 9ac511d
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 36 deletions.
@@ -62,7 +62,7 @@ bool QgsAnnotationManager::removeAnnotation( QgsAnnotation *annotation )

void QgsAnnotationManager::clear()
{
Q_FOREACH ( QgsAnnotation *a, mAnnotations )
for ( auto *a : qgsAsConst( mAnnotations ) )
{
removeAnnotation( a );
}
@@ -76,7 +76,7 @@ QList<QgsAnnotation *> QgsAnnotationManager::annotations() const
QList<QgsAnnotation *> QgsAnnotationManager::cloneAnnotations() const
{
QList<QgsAnnotation *> results;
Q_FOREACH ( const QgsAnnotation *a, mAnnotations )
for ( const auto *a : qgsAsConst( mAnnotations ) )
{
results << a->clone();
}
@@ -48,7 +48,7 @@ QString QgsAuthCertUtils::getSslProtocolName( QSsl::SslProtocol protocol )
QMap<QString, QSslCertificate> QgsAuthCertUtils::mapDigestToCerts( const QList<QSslCertificate> &certs )
{
QMap<QString, QSslCertificate> digestmap;
Q_FOREACH ( const QSslCertificate &cert, certs )
for ( const auto &cert : certs )
{
digestmap.insert( shaHexForCert( cert ), cert );
}
@@ -58,7 +58,7 @@ QMap<QString, QSslCertificate> QgsAuthCertUtils::mapDigestToCerts( const QList<Q
QMap<QString, QList<QSslCertificate> > QgsAuthCertUtils::certsGroupedByOrg( const QList<QSslCertificate> &certs )
{
QMap< QString, QList<QSslCertificate> > orgcerts;
Q_FOREACH ( const QSslCertificate &cert, certs )
for ( const auto &cert : certs )
{
QString org( SSL_SUBJECT_INFO( cert, QSslCertificate::Organization ) );
if ( org.isEmpty() )
@@ -72,7 +72,7 @@ QMap<QString, QList<QSslCertificate> > QgsAuthCertUtils::certsGroupedByOrg( cons
QMap<QString, QgsAuthConfigSslServer> QgsAuthCertUtils::mapDigestToSslConfigs( const QList<QgsAuthConfigSslServer> &configs )
{
QMap<QString, QgsAuthConfigSslServer> digestmap;
Q_FOREACH ( const QgsAuthConfigSslServer &config, configs )
for ( const auto &config : configs )
{
digestmap.insert( shaHexForCert( config.sslCertificate() ), config );
}
@@ -82,7 +82,7 @@ QMap<QString, QgsAuthConfigSslServer> QgsAuthCertUtils::mapDigestToSslConfigs( c
QMap<QString, QList<QgsAuthConfigSslServer> > QgsAuthCertUtils::sslConfigsGroupedByOrg( const QList<QgsAuthConfigSslServer> &configs )
{
QMap< QString, QList<QgsAuthConfigSslServer> > orgconfigs;
Q_FOREACH ( const QgsAuthConfigSslServer &config, configs )
for ( const auto &config : configs )
{
QString org( SSL_SUBJECT_INFO( config.sslCertificate(), QSslCertificate::Organization ) );

@@ -439,7 +439,7 @@ QCA::CertificateCollection QgsAuthCertUtils::qtCertsToQcaCollection( const QList
if ( QgsAuthManager::instance()->isDisabled() )
return qcacoll;

Q_FOREACH ( const QSslCertificate &cert, certs )
for ( const auto &cert : certs )
{
QCA::Certificate qcacert( qtCertToQcaCert( cert ) );
if ( !qcacert.isNull() )
@@ -622,8 +622,8 @@ QList<QgsAuthCertUtils::CertUsageType> QgsAuthCertUtils::certificateUsageTypes(
usages << QgsAuthCertUtils::CertAuthorityUsage;
}

QList<QCA::ConstraintType> certconsts = qcacert.constraints();
Q_FOREACH ( const QCA::ConstraintType &certconst, certconsts )
const QList<QCA::ConstraintType> certconsts = qcacert.constraints();
for ( const auto &certconst : certconsts )
{
if ( certconst.known() == QCA::KeyCertificateSign )
{
@@ -722,8 +722,8 @@ bool QgsAuthCertUtils::certificateIsSslServer( const QSslCertificate &cert )
return false;
}

QList<QCA::ConstraintType> certconsts = qcacert.constraints();
Q_FOREACH ( QCA::ConstraintType certconst, certconsts )
const QList<QCA::ConstraintType> certconsts = qcacert.constraints();
for ( const auto & certconst, certconsts )
{
if ( certconst.known() == QCA::KeyCertificateSign )
{
@@ -737,7 +737,7 @@ bool QgsAuthCertUtils::certificateIsSslServer( const QSslCertificate &cert )
bool serverauth = false;
bool dsignature = false;
bool keyencrypt = false;
Q_FOREACH ( QCA::ConstraintType certconst, certconsts )
for ( const auto &certconst : certconsts )
{
if ( certconst.known() == QCA::DigitalSignature )
{
@@ -791,7 +791,7 @@ bool QgsAuthCertUtils::certificateIsSslServer( const QSslCertificate &cert )
{
return false;
}
Q_FOREACH ( QCA::ConstraintType certconst, certconsts )
for ( const auto &certconst : certconsts )
{
if ( certconst.known() == QCA::EncipherOnly )
{
@@ -91,9 +91,9 @@ void QgsAuthMethodConfig::loadConfigString( const QString &configstr )
return;
}

QStringList confs( configstr.split( CONFIG_SEP ) );
const QStringList confs( configstr.split( CONFIG_SEP ) );

Q_FOREACH ( const QString &conf, confs )
for ( const auto &conf : confs )
{
if ( conf.contains( CONFIG_KEY_SEP ) )
{
@@ -255,7 +255,7 @@ const QgsPkiBundle QgsPkiBundle::fromPkcs12Paths( const QString &bundlepath,
QCA::KeyBundle bundle( QCA::KeyBundle::fromFile( bundlepath, passarray, &res, QStringLiteral( "qca-ossl" ) ) );
if ( res == QCA::ConvertGood && !bundle.isNull() )
{
QCA::CertificateChain cert_chain( bundle.certificateChain() );
const QCA::CertificateChain cert_chain( bundle.certificateChain() );
QSslCertificate cert( cert_chain.primary().toPEM().toLatin1() );
if ( !cert.isNull() )
{
@@ -270,7 +270,7 @@ const QgsPkiBundle QgsPkiBundle::fromPkcs12Paths( const QString &bundlepath,
if ( cert_chain.size() > 1 )
{
QList<QSslCertificate> ca_chain;
Q_FOREACH ( const QCA::Certificate &ca_cert, cert_chain )
for ( const auto &ca_cert : cert_chain )
{
if ( ca_cert != cert_chain.primary() )
{
@@ -366,7 +366,8 @@ QgsAuthConfigSslServer::QgsAuthConfigSslServer()
const QList<QSslError> QgsAuthConfigSslServer::sslIgnoredErrors() const
{
QList<QSslError> errors;
Q_FOREACH ( QSslError::SslError errenum, sslIgnoredErrorEnums() )
const QList<QSslError::SslError> ignoredErrors = sslIgnoredErrorEnums();
for ( QSslError::SslError errenum : ignoredErrors )
{
errors << QSslError( errenum );
}
@@ -381,7 +382,7 @@ const QString QgsAuthConfigSslServer::configString() const
configlist << QString::number( static_cast< int >( mSslProtocol ) );

QStringList errs;
Q_FOREACH ( const QSslError::SslError &err, mSslIgnoredErrors )
for ( auto err : mSslIgnoredErrors )
{
errs << QString::number( static_cast< int >( err ) );
}
@@ -408,8 +409,8 @@ void QgsAuthConfigSslServer::loadConfigString( const QString &config )
mSslProtocol = static_cast< QSsl::SslProtocol >( configlist.at( 2 ).toInt() );

mSslIgnoredErrors.clear();
QStringList errs( configlist.at( 3 ).split( QStringLiteral( "~~" ) ) );
Q_FOREACH ( const QString &err, errs )
const QStringList errs( configlist.at( 3 ).split( QStringLiteral( "~~" ) ) );
for ( const auto &err : errs )
{
mSslIgnoredErrors.append( static_cast< QSslError::SslError >( err.toInt() ) );
}
@@ -148,9 +148,9 @@ bool QgsAuthManager::init( const QString &pluginPath )
}

QgsDebugMsg( "Prioritizing qca-ossl over all other QCA providers..." );
QCA::ProviderList provds = QCA::providers();
const QCA::ProviderList provds = QCA::providers();
QStringList prlist;
Q_FOREACH ( QCA::Provider *p, provds )
for ( QCA::Provider *p : provds )
{
QString pn = p->name();
int pr = 0;
@@ -780,7 +780,8 @@ bool QgsAuthManager::registerCoreAuthMethods()

qDeleteAll( mAuthMethods );
mAuthMethods.clear();
Q_FOREACH ( const QString &authMethodKey, QgsAuthMethodRegistry::instance()->authMethodList() )
const QStringList methods = QgsAuthMethodRegistry::instance()->authMethodList();
for ( const auto &authMethodKey : methods )
{
mAuthMethods.insert( authMethodKey, QgsAuthMethodRegistry::instance()->authMethod( authMethodKey ).release() );
}
@@ -2086,7 +2087,7 @@ void QgsAuthManager::dumpIgnoredSslErrorsCache_()
while ( i != mIgnoredSslErrorsCache.constEnd() )
{
QStringList errs;
Q_FOREACH ( QSslError::SslError err, i.value() )
for ( auto err : i.value() )
{
errs << QgsAuthCertUtils::sslErrorEnumString( err );
}
@@ -2150,7 +2151,7 @@ bool QgsAuthManager::updateIgnoredSslErrorsCache( const QString &shahostport, co
}

QSet<QSslError::SslError> errs;
Q_FOREACH ( const QSslError &error, errors )
for ( const auto &error : errors )
{
if ( error.error() == QSslError::NoError )
continue;
@@ -2240,7 +2241,7 @@ bool QgsAuthManager::storeCertAuthorities( const QList<QSslCertificate> &certs )
return false;
}

Q_FOREACH ( const QSslCertificate &cert, certs )
for ( const auto &cert : certs )
{
if ( !storeCertAuthority( cert ) )
return false;
@@ -2410,7 +2411,7 @@ const QList<QSslCertificate> QgsAuthManager::getExtraFileCAs()
filecerts = QgsAuthCertUtils::certsFromFile( cafile );
}
// only CAs or certs capable of signing other certs are allowed
Q_FOREACH ( const QSslCertificate &cert, filecerts )
for ( const auto &cert : qgsAsConst( filecerts ) )
{
if ( !allowinvalid.toBool() && !cert.isValid() )
{
@@ -2547,7 +2548,7 @@ bool QgsAuthManager::removeCertTrustPolicies( const QList<QSslCertificate> &cert
return false;
}

Q_FOREACH ( const QSslCertificate &cert, certs )
for ( const auto &cert : certs )
{
if ( !removeCertTrustPolicy( cert ) )
return false;
@@ -2730,11 +2731,11 @@ bool QgsAuthManager::rebuildTrustedCaCertsCache()
const QByteArray QgsAuthManager::getTrustedCaCertsPemText()
{
QByteArray capem;
QList<QSslCertificate> certs( getTrustedCaCertsCache() );
const QList<QSslCertificate> certs( getTrustedCaCertsCache() );
if ( !certs.isEmpty() )
{
QStringList certslist;
Q_FOREACH ( const QSslCertificate &cert, certs )
for ( const auto &cert : certs )
{
certslist << cert.toPem();
}
@@ -2762,7 +2763,8 @@ void QgsAuthManager::clearAllCachedConfigs()
if ( isDisabled() )
return;

Q_FOREACH ( QString authcfg, configIds() )
const QStringList ids = configIds();
for ( const auto &authcfg : ids )
{
clearCachedConfig( authcfg );
}
@@ -3307,7 +3309,8 @@ bool QgsAuthManager::reencryptAllAuthenticationConfigs( const QString &prevpass,
return false;

bool res = true;
Q_FOREACH ( QString configid, configIds() )
const QStringList ids = configIds();
for ( const auto &configid : ids )
{
res = res && reencryptAuthenticationConfig( configid, prevpass, prevciv );
}
@@ -3395,7 +3398,7 @@ bool QgsAuthManager::reencryptAllAuthenticationSettings( const QString &prevpass
QStringList encryptedsettings;
encryptedsettings << "";

Q_FOREACH ( const QString &sett, encryptedsettings )
for ( const auto & sett, qgsAsConst( encryptedsettings ) )
{
if ( sett.isEmpty() || !existsAuthSetting( sett ) )
continue;
@@ -3468,7 +3471,8 @@ bool QgsAuthManager::reencryptAllAuthenticationIdentities( const QString &prevpa
return false;

bool res = true;
Q_FOREACH ( const QString &identid, getCertIdentityIds() )
const QStringList ids = getCertIdentityIds();
for ( const auto &identid : ids )
{
res = res && reencryptAuthenticationIdentity( identid, prevpass, prevciv );
}
@@ -3649,7 +3653,7 @@ bool QgsAuthManager::authDbTransactionQuery( QSqlQuery *query ) const

void QgsAuthManager::insertCaCertInCache( QgsAuthCertUtils::CaCertSource source, const QList<QSslCertificate> &certs )
{
Q_FOREACH ( const QSslCertificate &cert, certs )
for ( const auto &cert : certs )
{
mCaCertsCache.insert( QgsAuthCertUtils::shaHexForCert( cert ),
QPair<QgsAuthCertUtils::CaCertSource, QSslCertificate>( source, cert ) );

3 comments on commit 9ac511d

@m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented on 9ac511d Aug 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reasoning for flipping all type names to auto?
For reading / reviewing code as well as for navigating within the code in QtCreator I like it when the code is explicit.

@nyalldawson
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reasoning for flipping all type names to auto?

Seems like the modern c++ way to do things. I'm also not a huge fan, but want to stick by best practice c++11.

For reading / reviewing code as well as for navigating within the code in QtCreator I like it when the code is explicit.

If you set QtCreator to clang code model this works correctly (also unique_ptrs and other stuff)... but... it's also slow as hell.

I'm happy to change this, but can we set a general QGIS code style convention here? I'd rather one or the other than an inconsistent mix.

@m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented on 9ac511d Aug 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the modern c++ way to do things. I'm also not a huge fan, but want to stick by best practice c++11.

Probably because it's less typing for examples.

If you set QtCreator to clang code model this works correctly (also unique_ptrs and other stuff)... but... it's also slow as hell.

That's why I turned it off...

I'm happy to change this, but can we set a general QGIS code style convention here? I'd rather one or the other than an inconsistent mix.

Well, personally I've been using it to replace things like QList<QPair<int, QMap<QString, QgsSomeVeryVeryLongClassName>>>::const_iterator. I don't mind using it selectively in such situations.

Please sign in to comment.