Skip to content

Commit

Permalink
Fix #10744 (crashes with CSV files with parallel rendering)
Browse files Browse the repository at this point in the history
I can't replicate the crash, but the static QRegExp instances will not work
with multiple threads.
  • Loading branch information
wonder-sk committed Jul 16, 2014
1 parent 19708be commit 43635e7
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 12 deletions.
24 changes: 12 additions & 12 deletions src/providers/delimitedtext/qgsdelimitedtextfile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@
#include <QRegExp>
#include <QUrl>

static QString DefaultFieldName( "field_%1" );
static QRegExp InvalidFieldRegexp( "^\\d*(\\.\\d*)?$" );
// field_ is optional in following regexp to simplify QgsDelimitedTextFile::fieldNumber()
static QRegExp DefaultFieldRegexp( "^(?:field_)?(\\d+)$", Qt::CaseInsensitive );

QgsDelimitedTextFile::QgsDelimitedTextFile( QString url ) :
mFileName( QString() ),
Expand All @@ -52,7 +48,11 @@ QgsDelimitedTextFile::QgsDelimitedTextFile( QString url ) :
mRecordNumber( -1 ),
mHoldCurrentRecord( false ),
mMaxRecordNumber( -1 ),
mMaxFieldCount( 0 )
mMaxFieldCount( 0 ),
mDefaultFieldName( "field_%1" ),
mInvalidFieldRegexp( "^\\d*(\\.\\d*)?$" ),
// field_ is optional in following regexp to simplify QgsDelimitedTextFile::fieldNumber()
mDefaultFieldRegexp( "^(?:field_)?(\\d+)$", Qt::CaseInsensitive )
{
// The default type is CSV
setTypeCSV();
Expand Down Expand Up @@ -428,15 +428,15 @@ void QgsDelimitedTextFile::setFieldNames( const QStringList &names )
if ( name.length() > mMaxNameLength ) name = name.mid( 0, mMaxNameLength );

// If the name is invalid then reset it to default name
if ( InvalidFieldRegexp.exactMatch( name ) )
if ( mInvalidFieldRegexp.exactMatch( name ) )
{
name = DefaultFieldName.arg( fieldNo );
name = mDefaultFieldName.arg( fieldNo );
}
// If the name looks like a default field name (field_##), then it is
// valid if the number matches its column number..
else if ( DefaultFieldRegexp.indexIn( name ) == 0 )
else if ( mDefaultFieldRegexp.indexIn( name ) == 0 )
{
int col = DefaultFieldRegexp.capturedTexts()[1].toInt();
int col = mDefaultFieldRegexp.capturedTexts()[1].toInt();
nameOk = col == fieldNo;
}
// Otherwise it is valid if isn't the name of an existing field...
Expand Down Expand Up @@ -477,7 +477,7 @@ QStringList &QgsDelimitedTextFile::fieldNames()
{
for ( int i = mFieldNames.size() + 1; i <= mMaxFieldCount; i++ )
{
mFieldNames.append( DefaultFieldName.arg( i ) );
mFieldNames.append( mDefaultFieldName.arg( i ) );
}
}
return mFieldNames;
Expand All @@ -490,9 +490,9 @@ int QgsDelimitedTextFile::fieldIndex( QString name )
if ( mUseHeader && ! mFile ) reset();
// Try to determine the field based on a default field name, includes
// Field_### and simple integer fields.
if ( DefaultFieldRegexp.indexIn( name ) == 0 )
if ( mDefaultFieldRegexp.indexIn( name ) == 0 )
{
return DefaultFieldRegexp.capturedTexts()[1].toInt() - 1;
return mDefaultFieldRegexp.capturedTexts()[1].toInt() - 1;
}
for ( int i = 0; i < mFieldNames.size(); i++ )
{
Expand Down
4 changes: 4 additions & 0 deletions src/providers/delimitedtext/qgsdelimitedtextfile.h
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,10 @@ class QgsDelimitedTextFile : public QObject
// Maximum number of record (ie maximum record number visited)
long mMaxRecordNumber;
int mMaxFieldCount;

QString mDefaultFieldName;
QRegExp mInvalidFieldRegexp;
QRegExp mDefaultFieldRegexp;
};

#endif

1 comment on commit 43635e7

@gioman
Copy link
Contributor

@gioman gioman commented on 43635e7 Jul 17, 2014

Choose a reason for hiding this comment

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

backporting is possible?

Please sign in to comment.