Clean up state handling for ICU and iconv based codecs

Get rid of the hack for the FreeFunction and instead add a proper
function pointer to clear the data to the ConverterState struct.

Change-Id: I104aae1a4381c69f1a254713ec76e1aeaa862cdc
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
This commit is contained in:
Lars Knoll 2020-04-16 12:56:58 +02:00
parent a77b19a911
commit babcabfbc8
6 changed files with 39 additions and 40 deletions

View File

@ -121,7 +121,7 @@ void QIconvCodec::IconvState::saveChars(const char *c, int count)
static void qIconvCodecStateFree(QTextCodec::ConverterState *state) static void qIconvCodecStateFree(QTextCodec::ConverterState *state)
{ {
delete reinterpret_cast<QIconvCodec::IconvState *>(state->d); delete reinterpret_cast<QIconvCodec::IconvState *>(state->d[0]);
} }
Q_GLOBAL_STATIC(QThreadStorage<QIconvCodec::IconvState *>, toUnicodeState) Q_GLOBAL_STATIC(QThreadStorage<QIconvCodec::IconvState *>, toUnicodeState)
@ -139,15 +139,14 @@ QString QIconvCodec::convertToUnicode(const char* chars, int len, ConverterState
if (convState) { if (convState) {
// stateful conversion // stateful conversion
pstate = reinterpret_cast<IconvState **>(&convState->d); pstate = reinterpret_cast<IconvState **>(&convState->d[0]);
if (convState->d) { if (convState->d[0]) {
// restore state // restore state
remainingCount = convState->remainingChars; remainingCount = convState->remainingChars;
remainingBuffer = (*pstate)->buffer; remainingBuffer = (*pstate)->buffer;
} else { } else {
// first time // first time
convState->flags |= FreeFunction; convState->clearFn = qIconvCodecStateFree;
QTextCodecUnalignedPointer::encode(convState->state_data, qIconvCodecStateFree);
} }
} else { } else {
QThreadStorage<QIconvCodec::IconvState *> *ts = toUnicodeState(); QThreadStorage<QIconvCodec::IconvState *> *ts = toUnicodeState();

View File

@ -60,7 +60,7 @@ typedef QList<QByteArray>::ConstIterator ByteArrayListConstIt;
static void qIcuCodecStateFree(QTextCodec::ConverterState *state) static void qIcuCodecStateFree(QTextCodec::ConverterState *state)
{ {
ucnv_close(static_cast<UConverter *>(state->d)); ucnv_close(static_cast<UConverter *>(state->d[0]));
} }
bool qTextCodecNameMatch(const char *n, const char *h) bool qTextCodecNameMatch(const char *n, const char *h)
@ -569,18 +569,17 @@ UConverter *QIcuCodec::getConverter(QTextCodec::ConverterState *state) const
{ {
UConverter *conv = nullptr; UConverter *conv = nullptr;
if (state) { if (state) {
if (!state->d) { if (!state->d[0]) {
// first time // first time
state->flags |= QTextCodec::FreeFunction; state->clearFn = qIcuCodecStateFree;
QTextCodecUnalignedPointer::encode(state->state_data, qIcuCodecStateFree);
UErrorCode error = U_ZERO_ERROR; UErrorCode error = U_ZERO_ERROR;
state->d = ucnv_open(m_name, &error); state->d[0] = ucnv_open(m_name, &error);
ucnv_setSubstChars(static_cast<UConverter *>(state->d), ucnv_setSubstChars(static_cast<UConverter *>(state->d[0]),
state->flags & QTextCodec::ConvertInvalidToNull ? "\0" : "?", 1, &error); state->flags & QTextCodec::ConvertInvalidToNull ? "\0" : "?", 1, &error);
if (U_FAILURE(error)) if (U_FAILURE(error))
qDebug("getConverter(state) ucnv_open failed %s %s", m_name, u_errorName(error)); qDebug("getConverter(state) ucnv_open failed %s %s", m_name, u_errorName(error));
} }
conv = static_cast<UConverter *>(state->d); conv = static_cast<UConverter *>(state->d[0]);
} }
if (!conv) { if (!conv) {
// stateless conversion // stateless conversion

View File

@ -343,10 +343,19 @@ static void setup() {}
*/ */
QTextCodec::ConverterState::~ConverterState() QTextCodec::ConverterState::~ConverterState()
{ {
if (flags & FreeFunction) clear();
(QTextCodecUnalignedPointer::decode(state_data))(this); }
else if (d)
free(d); void QTextCodec::ConverterState::clear()
{
if (clearFn)
clearFn(this);
remainingChars = 0;
invalidChars = 0;
state_data[0] = 0;
state_data[1] = 0;
state_data[2] = 0;
state_data[3] = 0;
} }
/*! /*!

View File

@ -92,20 +92,25 @@ public:
enum ConversionFlag { enum ConversionFlag {
DefaultConversion, DefaultConversion,
ConvertInvalidToNull = 0x80000000, ConvertInvalidToNull = 0x80000000,
IgnoreHeader = 0x1, IgnoreHeader = 0x1
FreeFunction = 0x2
}; };
Q_DECLARE_FLAGS(ConversionFlags, ConversionFlag) Q_DECLARE_FLAGS(ConversionFlags, ConversionFlag)
struct Q_CORE_EXPORT ConverterState { struct Q_CORE_EXPORT ConverterState {
ConverterState(ConversionFlags f = DefaultConversion) ConverterState(ConversionFlags f = DefaultConversion)
: flags(f), remainingChars(0), invalidChars(0), d(nullptr) { state_data[0] = state_data[1] = state_data[2] = 0; } : flags(f), state_data{0, 0, 0, 0} {}
~ConverterState(); ~ConverterState();
ConversionFlags flags; ConversionFlags flags;
int remainingChars; int remainingChars = 0;
int invalidChars; int invalidChars = 0;
uint state_data[3];
void *d; union {
uint state_data[4];
void *d[2];
};
void clear();
using ClearDataFn = void (*)(ConverterState *);
ClearDataFn clearFn = nullptr;
private: private:
Q_DISABLE_COPY(ConverterState) Q_DISABLE_COPY(ConverterState)
}; };

View File

@ -66,21 +66,6 @@ QT_BEGIN_NAMESPACE
typedef void (*QTextCodecStateFreeFunction)(QTextCodec::ConverterState*); typedef void (*QTextCodecStateFreeFunction)(QTextCodec::ConverterState*);
struct QTextCodecUnalignedPointer
{
static inline QTextCodecStateFreeFunction decode(const uint *src)
{
quintptr data;
memcpy(&data, src, sizeof(data));
return reinterpret_cast<QTextCodecStateFreeFunction>(data);
}
static inline void encode(uint *dst, QTextCodecStateFreeFunction fn)
{
quintptr data = reinterpret_cast<quintptr>(fn);
memcpy(dst, &data, sizeof(data));
}
};
bool qTextCodecNameMatch(const char *a, const char *b); bool qTextCodecNameMatch(const char *a, const char *b);
#else // without textcodec: #else // without textcodec:

View File

@ -364,12 +364,14 @@ static void copyConverterStateHelper(QTextCodec::ConverterState *dest,
{ {
// ### QTextCodec::ConverterState's copy constructors and assignments are // ### QTextCodec::ConverterState's copy constructors and assignments are
// private. This function copies the structure manually. // private. This function copies the structure manually.
Q_ASSERT(!src->d); Q_ASSERT(!src->clearFn);
dest->flags = src->flags; dest->flags = src->flags;
dest->remainingChars = src->remainingChars;
dest->invalidChars = src->invalidChars; dest->invalidChars = src->invalidChars;
dest->state_data[0] = src->state_data[0]; dest->state_data[0] = src->state_data[0];
dest->state_data[1] = src->state_data[1]; dest->state_data[1] = src->state_data[1];
dest->state_data[2] = src->state_data[2]; dest->state_data[2] = src->state_data[2];
dest->state_data[3] = src->state_data[3];
} }
#endif #endif
@ -787,7 +789,7 @@ inline void QTextStreamPrivate::consume(int size)
inline void QTextStreamPrivate::saveConverterState(qint64 newPos) inline void QTextStreamPrivate::saveConverterState(qint64 newPos)
{ {
#if QT_CONFIG(textcodec) #if QT_CONFIG(textcodec)
if (readConverterState.d) { if (readConverterState.clearFn) {
// converter cannot be copied, so don't save anything // converter cannot be copied, so don't save anything
// don't update readBufferStartDevicePos either // don't update readBufferStartDevicePos either
return; return;