Navigation Menu

Skip to content

Commit

Permalink
Flip a couple of Q_FOREACHs to c++11 for loops
Browse files Browse the repository at this point in the history 
... 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
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.
4 changes: 2 additions & 2 deletions src/core/annotations/qgsannotationmanager.cpp
View file
Expand Up @@ -62,7 +62,7 @@ bool QgsAnnotationManager::removeAnnotation( QgsAnnotation *annotation )

void QgsAnnotationManager::clear()
{
Q_FOREACH ( QgsAnnotation *a, mAnnotations )
for ( auto *a : qgsAsConst( mAnnotations ) )
{
removeAnnotation( a );
}
Expand All @@ -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();
}
Expand Down
22 changes: 11 additions & 11 deletions src/core/auth/qgsauthcertutils.cpp
View file
Expand Up @@ -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 );
}
Expand All @@ -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() )
Expand All @@ -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 );
}
Expand All @@ -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 ) );

Expand Down Expand Up @@ -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() )
Expand Down Expand Up @@ -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 )
{
Expand Down Expand Up @@ -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 )
{
Expand All @@ -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 )
{
Expand Down Expand Up @@ -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 )
{
Expand Down
17 changes: 9 additions & 8 deletions src/core/auth/qgsauthconfig.cpp
View file
Expand Up @@ -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 ) )
{
Expand Down Expand Up @@ -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() )
{
Expand All @@ -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() )
{
Expand Down Expand Up @@ -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 );
}
Expand All @@ -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 ) );
}
Expand All @@ -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() ) );
}
Expand Down
34 changes: 19 additions & 15 deletions src/core/auth/qgsauthmanager.cpp
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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() );
}
Expand Down Expand Up @@ -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 );
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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() )
{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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 );
}
Expand Down Expand Up @@ -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 );
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 );
}
Expand Down Expand Up @@ -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 ) );
Expand Down

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.