From 3a218217f41e0aa8d8a8b95cf3865ac676106c85 Mon Sep 17 00:00:00 2001 From: James Urquhart Date: Fri, 15 May 2015 12:27:52 +0100 Subject: [PATCH 1/3] Add workaround for issue #1292 --- Engine/source/console/console.cpp | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/Engine/source/console/console.cpp b/Engine/source/console/console.cpp index 81c460d79..dd25ef399 100644 --- a/Engine/source/console/console.cpp +++ b/Engine/source/console/console.cpp @@ -1759,12 +1759,32 @@ const char *ConsoleValue::getStringValue() return sval; else if (type == TypeInternalStringStackPtr) return STR.mBuffer + (uintptr_t)sval; - if(type == TypeInternalFloat) - return Con::getData(TypeF32, &fval, 0); - else if(type == TypeInternalInt) - return Con::getData(TypeS32, &ival, 0); else - return Con::getData(type, dataPtr, 0, enumTable); + { + // We need a string representation, so lets create one + const char *internalValue = NULL; + + if(type == TypeInternalFloat) + internalValue = Con::getData(TypeF32, &fval, 0); + else if(type == TypeInternalInt) + internalValue = Con::getData(TypeS32, &ival, 0); + else + internalValue = Con::getData(type, dataPtr, 0, enumTable); + + if (!internalValue) + return ""; + + U32 stringLen = dStrlen(internalValue); + U32 newLen = ((stringLen + 1) + 15) & ~15; // pad upto next cache line + + if(sval == typeValueEmpty || type == TypeInternalStackString || type == TypeInternalStringStackPtr) + sval = (char *) dMalloc(newLen); + else if(newLen > bufferLen) + sval = (char *) dRealloc(sval, newLen); + + dStrcpy(sval, internalValue); + return sval; + } } StringStackPtr ConsoleValue::getStringStackPtr() From 15169eab9fcc0b750f6ba3365506b9816744990e Mon Sep 17 00:00:00 2001 From: James Urquhart Date: Sat, 16 May 2015 14:50:20 +0100 Subject: [PATCH 2/3] Fix issue with registered variables becoming corrupted when string value was accessed. --- Engine/source/console/console.cpp | 21 ++++++++----- Engine/source/console/console.h | 9 +++--- Engine/source/console/consoleInternal.cpp | 37 +++++++++++++++-------- 3 files changed, 43 insertions(+), 24 deletions(-) diff --git a/Engine/source/console/console.cpp b/Engine/source/console/console.cpp index dd25ef399..b12cf022a 100644 --- a/Engine/source/console/console.cpp +++ b/Engine/source/console/console.cpp @@ -1769,7 +1769,7 @@ const char *ConsoleValue::getStringValue() else if(type == TypeInternalInt) internalValue = Con::getData(TypeS32, &ival, 0); else - internalValue = Con::getData(type, dataPtr, 0, enumTable); + return Con::getData(type, dataPtr, 0, enumTable); // We can't save sval here since it is the same as dataPtr if (!internalValue) return ""; @@ -1777,12 +1777,14 @@ const char *ConsoleValue::getStringValue() U32 stringLen = dStrlen(internalValue); U32 newLen = ((stringLen + 1) + 15) & ~15; // pad upto next cache line - if(sval == typeValueEmpty || type == TypeInternalStackString || type == TypeInternalStringStackPtr) + if (bufferLen == 0) sval = (char *) dMalloc(newLen); else if(newLen > bufferLen) sval = (char *) dRealloc(sval, newLen); dStrcpy(sval, internalValue); + bufferLen = newLen; + return sval; } } @@ -1820,11 +1822,13 @@ void ConsoleValue::setIntValue(U32 val) { fval = (F32)val; ival = val; - if(sval != typeValueEmpty) + if(bufferLen > 0) { - if (type != TypeInternalStackString && type != TypeInternalStringStackPtr) dFree(sval); - sval = typeValueEmpty; + dFree(sval); + bufferLen = 0; } + + sval = typeValueEmpty; type = TypeInternalInt; } else @@ -1845,11 +1849,12 @@ void ConsoleValue::setFloatValue(F32 val) { fval = val; ival = static_cast(val); - if(sval != typeValueEmpty) + if(bufferLen > 0) { - if (type != TypeInternalStackString && type != TypeInternalStringStackPtr) dFree(sval); - sval = typeValueEmpty; + dFree(sval); + bufferLen = 0; } + sval = typeValueEmpty; type = TypeInternalFloat; } else diff --git a/Engine/source/console/console.h b/Engine/source/console/console.h index 4ca0c8da8..8ca1b26b4 100644 --- a/Engine/source/console/console.h +++ b/Engine/source/console/console.h @@ -186,19 +186,20 @@ public: fval = 0; sval = typeValueEmpty; bufferLen = 0; - type = TypeInternalString; + type = TypeInternalString; } void cleanup() { - if (type <= TypeInternalString && - sval != typeValueEmpty && type != TypeInternalStackString && type != TypeInternalStringStackPtr) + if (bufferLen > 0) + { dFree(sval); + bufferLen = 0; + } sval = typeValueEmpty; type = ConsoleValue::TypeInternalString; ival = 0; fval = 0; - bufferLen = 0; } }; diff --git a/Engine/source/console/consoleInternal.cpp b/Engine/source/console/consoleInternal.cpp index e72ddee80..49398d869 100644 --- a/Engine/source/console/consoleInternal.cpp +++ b/Engine/source/console/consoleInternal.cpp @@ -512,13 +512,17 @@ void ConsoleValue::setStringValue(const char * value) */ if (value == typeValueEmpty) { - if (sval && sval != typeValueEmpty && type != TypeInternalStackString && type != TypeInternalStringStackPtr) dFree(sval); - sval = typeValueEmpty; + if (bufferLen > 0) + { + dFree(sval); bufferLen = 0; - fval = 0.f; - ival = 0; - type = TypeInternalString; - return; + } + + sval = typeValueEmpty; + fval = 0.f; + ival = 0; + type = TypeInternalString; + return; } U32 stringLen = dStrlen(value); @@ -541,7 +545,7 @@ void ConsoleValue::setStringValue(const char * value) // may as well pad to the next cache line U32 newLen = ((stringLen + 1) + 15) & ~15; - if(sval == typeValueEmpty || type == TypeInternalStackString || type == TypeInternalStringStackPtr) + if(bufferLen == 0) sval = (char *) dMalloc(newLen); else if(newLen > bufferLen) sval = (char *) dRealloc(sval, newLen); @@ -562,11 +566,16 @@ void ConsoleValue::setStackStringValue(const char *value) if(type <= ConsoleValue::TypeInternalString) { + // sval might still be temporarily present so we need to check and free it + if (bufferLen > 0) + { + dFree(sval); + bufferLen = 0; + } + if (value == typeValueEmpty) { - if (sval && sval != typeValueEmpty && type != ConsoleValue::TypeInternalStackString && type != ConsoleValue::TypeInternalStringStackPtr) dFree(sval); sval = typeValueEmpty; - bufferLen = 0; fval = 0.f; ival = 0; type = TypeInternalString; @@ -587,7 +596,7 @@ void ConsoleValue::setStackStringValue(const char *value) type = TypeInternalStackString; sval = (char*)value; - bufferLen = stringLen; + bufferLen = 0; } else Con::setData(type, dataPtr, 0, 1, &value, enumTable); @@ -598,7 +607,11 @@ void ConsoleValue::setStringStackPtrValue(StringStackPtr ptrValue) if(type <= ConsoleValue::TypeInternalString) { const char *value = StringStackPtrRef(ptrValue).getPtr(&STR); - if (sval && sval != typeValueEmpty && type != ConsoleValue::TypeInternalStackString && type != TypeInternalStringStackPtr) dFree(sval); + if (bufferLen > 0) + { + dFree(sval); + bufferLen = 0; + } U32 stringLen = dStrlen(value); if(stringLen < 256) @@ -614,7 +627,7 @@ void ConsoleValue::setStringStackPtrValue(StringStackPtr ptrValue) type = TypeInternalStringStackPtr; sval = (char*)(value - STR.mBuffer); - bufferLen = stringLen; + bufferLen = 0; } else { From e5c28b4d7f8d85a20d4497bae3f5386fe4084d40 Mon Sep 17 00:00:00 2001 From: James Urquhart Date: Sat, 16 May 2015 14:57:40 +0100 Subject: [PATCH 3/3] Simplify buffer check when adding a registered variable --- Engine/source/console/consoleInternal.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Engine/source/console/consoleInternal.cpp b/Engine/source/console/consoleInternal.cpp index 49398d869..044e5238b 100644 --- a/Engine/source/console/consoleInternal.cpp +++ b/Engine/source/console/consoleInternal.cpp @@ -693,8 +693,7 @@ Dictionary::Entry* Dictionary::addVariable( const char *name, Entry *ent = add(StringTable->insert(name)); if ( ent->value.type <= ConsoleValue::TypeInternalString && - ent->value.sval != typeValueEmpty && - ent->value.type != ConsoleValue::TypeInternalStackString && ent->value.type != ConsoleValue::TypeInternalStringStackPtr ) + ent->value.bufferLen > 0 ) dFree(ent->value.sval); ent->value.type = type;