From b3759768e4f2ca09b1c227b6f31d3586c9685549 Mon Sep 17 00:00:00 2001 From: Eugene Golushkov Date: Tue, 27 Aug 2024 15:02:46 +0200 Subject: [PATCH] Use std::snprintf() rather than sprintf(). Use specifically std version to catch #define snprintf _snprintf during compile time, as we want standard always NULL terminating behavior. Fixed asserts limits in OgreLwString.h --- OgreMain/include/OgreIdString.h | 62 +++----------------- OgreMain/include/OgreLwConstString.h | 11 +--- OgreMain/include/OgreLwString.h | 87 +++++++++++----------------- OgreMain/src/OgreAssert.cpp | 18 +++--- OgreMain/src/OgreHlms.cpp | 4 +- OgreMain/src/OgrePass.cpp | 5 +- OgreMain/src/OgreSearchOps.cpp | 7 ++- 7 files changed, 61 insertions(+), 133 deletions(-) diff --git a/OgreMain/include/OgreIdString.h b/OgreMain/include/OgreIdString.h index 3c5ae626658..2131b3b1340 100644 --- a/OgreMain/include/OgreIdString.h +++ b/OgreMain/include/OgreIdString.h @@ -32,8 +32,8 @@ THE SOFTWARE. #include "OgrePrerequisites.h" #include // PRIx64 -#include // sprintf #include // strlen +#include // std::snprintf #include #include "Hash/MurmurHash3.h" @@ -145,10 +145,6 @@ namespace Ogre } #if OGRE_DEBUG_MODE >= OGRE_DEBUG_MEDIUM || OGRE_IDSTRING_ALWAYS_READABLE -# if OGRE_COMPILER == OGRE_COMPILER_MSVC -# pragma warning( push ) -# pragma warning( disable : 4996 ) // Unsecure CRT deprecation warning -# endif void OGRE_COPY_DEBUG_STRING( const char *string ) { @@ -184,8 +180,7 @@ namespace Ogre void OGRE_COPY_DEBUG_STRING( uint32 value ) { - sprintf( mDebugString, "[Value 0x%.8x]", value ); - mDebugString[OGRE_DEBUG_STR_SIZE - 1] = '\0'; + std::snprintf( mDebugString, OGRE_DEBUG_STR_SIZE, "[Value 0x%.8x]", value ); } void OGRE_APPEND_DEBUG_STRING( const char *string ) @@ -217,9 +212,6 @@ namespace Ogre } } -# if OGRE_COMPILER == OGRE_COMPILER_MSVC -# pragma warning( pop ) -# endif #endif void operator+=( IdString idString ) @@ -308,23 +300,14 @@ namespace Ogre /// Always returns "[Hash 0x0a0100ef]" strings in any mode std::string getReleaseText() const { -#if OGRE_COMPILER == OGRE_COMPILER_MSVC -# pragma warning( push ) -# pragma warning( disable : 4996 ) // Unsecure CRT deprecation warning -#endif - - char tmp[( OGRE_HASH_BITS >> 2 ) + 10]; + const size_t tmplen = ( OGRE_HASH_BITS >> 2 ) + 10; + char tmp[tmplen]; #ifdef OGRE_IDSTRING_USE_128 - sprintf( tmp, "[Hash 0x%.16" PRIx64 "%.16" PRIx64 "]", mHash[0], mHash[1] ); + std::snprintf( tmp, tmplen, "[Hash 0x%.16" PRIx64 "%.16" PRIx64 "]", mHash[0], mHash[1] ); #else - sprintf( tmp, "[Hash 0x%.8x]", mHash ); + std::snprintf( tmp, tmplen, "[Hash 0x%.8x]", mHash ); #endif - tmp[( OGRE_HASH_BITS >> 2 ) + 10 - 1] = '\0'; return std::string( tmp ); - -#if OGRE_COMPILER == OGRE_COMPILER_MSVC -# pragma warning( pop ) -#endif } /** C String version. Zero allocations. @@ -347,38 +330,11 @@ namespace Ogre /// C String version. Zero allocations. See getFriendlyText. void getReleaseText( char *outCStr, size_t stringSize ) const { -#if OGRE_COMPILER == OGRE_COMPILER_MSVC -# pragma warning( push ) -# pragma warning( disable : 4996 ) // Unsecure CRT deprecation warning -#endif - - if( stringSize < ( OGRE_HASH_BITS >> 2u ) + 10u ) - { - // Not big enough. Use a temp buffer and then copy + truncate. - char tmp[( OGRE_HASH_BITS >> 2 ) + 10]; #ifdef OGRE_IDSTRING_USE_128 - sprintf( tmp, "[Hash 0x%.16" PRIx64 "%.16" PRIx64 "]", mHash[0], mHash[1] ); + std::snprintf( outCStr, stringSize, "[Hash 0x%.16" PRIx64 "%.16" PRIx64 "]", mHash[0], + mHash[1] ); #else - sprintf( tmp, "[Hash 0x%.8x]", mHash ); -#endif - tmp[( OGRE_HASH_BITS >> 2 ) + 10 - 1] = '\0'; - - memcpy( outCStr, tmp, stringSize ); - outCStr[stringSize - 1u] = '\0'; - } - else - { -// Write directly to the output buffer. It's big enough. -#ifdef OGRE_IDSTRING_USE_128 - sprintf( outCStr, "[Hash 0x%.16" PRIx64 "%.16" PRIx64 "]", mHash[0], mHash[1] ); -#else - sprintf( outCStr, "[Hash 0x%.8x]", mHash ); -#endif - outCStr[( OGRE_HASH_BITS >> 2 ) + 10 - 1] = '\0'; - } - -#if OGRE_COMPILER == OGRE_COMPILER_MSVC -# pragma warning( pop ) + std::snprintf( outCStr, stringSize, "[Hash 0x%.8x]", mHash ); #endif } diff --git a/OgreMain/include/OgreLwConstString.h b/OgreMain/include/OgreLwConstString.h index 16f25fbd1f0..db694d951f6 100644 --- a/OgreMain/include/OgreLwConstString.h +++ b/OgreMain/include/OgreLwConstString.h @@ -38,16 +38,11 @@ #include #include #include +#include #if !defined( _MSC_VER ) || ( _MSC_VER > 1600 ) # include #endif -#include - -#ifdef _MSC_VER -# pragma warning( push ) // CRT deprecation -# pragma warning( disable : 4996 ) -#endif namespace Ogre { @@ -179,8 +174,4 @@ namespace Ogre }; } // namespace Ogre -#ifdef _MSC_VER -# pragma warning( pop ) -#endif - #endif diff --git a/OgreMain/include/OgreLwString.h b/OgreMain/include/OgreLwString.h index 2f9c2ebe539..7815712a184 100644 --- a/OgreMain/include/OgreLwString.h +++ b/OgreMain/include/OgreLwString.h @@ -43,14 +43,6 @@ # include #endif -#ifndef _MSC_VER -# define OGRE_LWSTRING_SNPRINTF_DEFINED -# define _snprintf snprintf -#else -# pragma warning( push ) // CRT deprecation -# pragma warning( disable : 4996 ) -#endif - namespace Ogre { /** This is a Light-weight string wrapper around the C string interface. @@ -60,7 +52,7 @@ namespace Ogre The goals are: + No dynamic allocation. - + Easier to read and control than sprintf() + + Easier to read and control than snprintf() + Type-safe (in as much as C can ever be type-safe - bloody auto-converts). + Overflow-safe (i.e. it will refuse to scribble, and will assert in debug mode). @@ -247,37 +239,33 @@ namespace Ogre LwString &a( int32 a0 ) { - int written = _snprintf( mStrPtr + mSize, mCapacity - mSize, "%i", a0 ); - assert( ( written >= 0 ) && ( (size_t)written < mCapacity ) ); - mStrPtr[mCapacity - 1] = '\0'; - mSize = std::min( mSize + (size_t)std::max( written, 0 ), mCapacity - 1u ); + int required = std::snprintf( mStrPtr + mSize, mCapacity - mSize, "%i", a0 ); + assert( ( required >= 0 ) && ( (size_t)required < mCapacity - mSize ) ); + mSize = std::min( mSize + (size_t)std::max( required, 0 ), mCapacity - 1u ); return *this; } LwString &a( uint32 a0 ) { - int written = _snprintf( mStrPtr + mSize, mCapacity - mSize, "%u", a0 ); - assert( ( written >= 0 ) && ( (size_t)written < mCapacity ) ); - mStrPtr[mCapacity - 1] = '\0'; - mSize = std::min( mSize + (size_t)std::max( written, 0 ), mCapacity - 1u ); + int required = std::snprintf( mStrPtr + mSize, mCapacity - mSize, "%u", a0 ); + assert( ( required >= 0 ) && ( (size_t)required < mCapacity - mSize ) ); + mSize = std::min( mSize + (size_t)std::max( required, 0 ), mCapacity - 1u ); return *this; } LwString &a( int64 a0 ) { - int written = _snprintf( mStrPtr + mSize, mCapacity - mSize, "%" PRIi64, a0 ); - assert( ( written >= 0 ) && ( (size_t)written < mCapacity ) ); - mStrPtr[mCapacity - 1] = '\0'; - mSize = std::min( mSize + (size_t)std::max( written, 0 ), mCapacity - 1u ); + int required = std::snprintf( mStrPtr + mSize, mCapacity - mSize, "%" PRIi64, a0 ); + assert( ( required >= 0 ) && ( (size_t)required < mCapacity - mSize ) ); + mSize = std::min( mSize + (size_t)std::max( required, 0 ), mCapacity - 1u ); return *this; } LwString &a( uint64 a0 ) { - int written = _snprintf( mStrPtr + mSize, mCapacity - mSize, "%" PRIu64, a0 ); - assert( ( written >= 0 ) && ( (size_t)written < mCapacity ) ); - mStrPtr[mCapacity - 1] = '\0'; - mSize = std::min( mSize + (size_t)std::max( written, 0 ), mCapacity - 1u ); + int required = std::snprintf( mStrPtr + mSize, mCapacity - mSize, "%" PRIu64, a0 ); + assert( ( required >= 0 ) && ( (size_t)required < mCapacity - mSize ) ); + mSize = std::min( mSize + (size_t)std::max( required, 0 ), mCapacity - 1u ); return *this; } @@ -317,36 +305,36 @@ namespace Ogre LwString &a( Float a0 ) { - int written = -1; + int required = -1; if( a0.mMinWidth < 0 ) { if( a0.mPrecision < 0 ) { - written = _snprintf( mStrPtr + mSize, mCapacity - mSize, "%f", (double)a0.mValue ); + required = + std::snprintf( mStrPtr + mSize, mCapacity - mSize, "%f", (double)a0.mValue ); } else { - written = _snprintf( mStrPtr + mSize, mCapacity - mSize, "%.*f", a0.mPrecision, - (double)a0.mValue ); + required = std::snprintf( mStrPtr + mSize, mCapacity - mSize, "%.*f", a0.mPrecision, + (double)a0.mValue ); } } else { if( a0.mPrecision < 0 ) { - written = _snprintf( mStrPtr + mSize, mCapacity - mSize, "%*f", a0.mMinWidth, - (double)a0.mValue ); + required = std::snprintf( mStrPtr + mSize, mCapacity - mSize, "%*f", a0.mMinWidth, + (double)a0.mValue ); } else { - written = _snprintf( mStrPtr + mSize, mCapacity - mSize, "%*.*f", a0.mMinWidth, - a0.mPrecision, (double)a0.mValue ); + required = std::snprintf( mStrPtr + mSize, mCapacity - mSize, "%*.*f", a0.mMinWidth, + a0.mPrecision, (double)a0.mValue ); } } - mStrPtr[mCapacity - 1] = '\0'; - assert( ( written >= 0 ) && ( (unsigned)written < mCapacity ) ); - mSize = std::min( mSize + (size_t)std::max( written, 0 ), mCapacity - 1u ); + assert( ( required >= 0 ) && ( (unsigned)required < mCapacity - mSize ) ); + mSize = std::min( mSize + (size_t)std::max( required, 0 ), mCapacity - 1u ); return *this; } @@ -386,36 +374,35 @@ namespace Ogre LwString &a( Double a0 ) { - int written = -1; + int required = -1; if( a0.mMinWidth < 0 ) { if( a0.mPrecision < 0 ) { - written = _snprintf( mStrPtr + mSize, mCapacity - mSize, "%lf", a0.mValue ); + required = std::snprintf( mStrPtr + mSize, mCapacity - mSize, "%lf", a0.mValue ); } else { - written = _snprintf( mStrPtr + mSize, mCapacity - mSize, "%.*lf", a0.mPrecision, - a0.mValue ); + required = std::snprintf( mStrPtr + mSize, mCapacity - mSize, "%.*lf", a0.mPrecision, + a0.mValue ); } } else { if( a0.mPrecision < 0 ) { - written = - _snprintf( mStrPtr + mSize, mCapacity - mSize, "%*lf", a0.mMinWidth, a0.mValue ); + required = std::snprintf( mStrPtr + mSize, mCapacity - mSize, "%*lf", a0.mMinWidth, + a0.mValue ); } else { - written = _snprintf( mStrPtr + mSize, mCapacity - mSize, "%*.*lf", a0.mMinWidth, - a0.mPrecision, a0.mValue ); + required = std::snprintf( mStrPtr + mSize, mCapacity - mSize, "%*.*lf", a0.mMinWidth, + a0.mPrecision, a0.mValue ); } } - mStrPtr[mCapacity - 1] = '\0'; - assert( ( written >= 0 ) && ( (unsigned)written < mCapacity ) ); - mSize = std::min( mSize + static_cast( std::max( written, 0 ) ), + assert( ( required >= 0 ) && ( (unsigned)required < mCapacity - mSize ) ); + mSize = std::min( mSize + static_cast( std::max( required, 0 ) ), static_cast( mCapacity - 1u ) ); return *this; } @@ -493,10 +480,4 @@ namespace Ogre }; } // namespace Ogre -#ifdef OGRE_LWSTRING_SNPRINTF_DEFINED -# undef _snprintf -#else -# pragma warning( pop ) -#endif - #endif diff --git a/OgreMain/src/OgreAssert.cpp b/OgreMain/src/OgreAssert.cpp index b2314321625..ad276158655 100644 --- a/OgreMain/src/OgreAssert.cpp +++ b/OgreMain/src/OgreAssert.cpp @@ -44,7 +44,7 @@ #include "OgreAssert.h" #include -#include +#include // std::snprintf namespace Ogre { @@ -60,18 +60,18 @@ namespace Ogre char path[MAX_PATH]; GetModuleFileNameA( NULL, path, MAX_PATH ); - _snprintf( fullmsg, sizeof( fullmsg ), - "Assertion failed!\n\n" - "Program: %s\n" - "File: %s\n" - "Line: %d\n", - path, file, line ); + std::snprintf( fullmsg, sizeof( fullmsg ), + "Assertion failed!\n\n" + "Program: %s\n" + "File: %s\n" + "Line: %d\n", + path, file, line ); if( condition != NULL ) - _snprintf( fullmsg, sizeof( fullmsg ), "%s\nExpresson: %s\n", fullmsg, condition ); + std::snprintf( fullmsg, sizeof( fullmsg ), "%s\nExpresson: %s\n", fullmsg, condition ); if( msg != NULL ) - _snprintf( fullmsg, sizeof( fullmsg ), "%s\n%s\n", fullmsg, msg ); + std::snprintf( fullmsg, sizeof( fullmsg ), "%s\n%s\n", fullmsg, msg ); strncat( fullmsg, "\n\n(Press Retry to debug application)", sizeof( fullmsg ) ); diff --git a/OgreMain/src/OgreHlms.cpp b/OgreMain/src/OgreHlms.cpp index 0fb16e2c159..347cc475603 100644 --- a/OgreMain/src/OgreHlms.cpp +++ b/OgreMain/src/OgreHlms.cpp @@ -1100,7 +1100,7 @@ namespace Ogre if( subString.startWith( counterVar ) ) { char tmp[16]; - sprintf( tmp, "%lu", (unsigned long)passNum ); + std::snprintf( tmp, sizeof( tmp ), "%lu", (unsigned long)passNum ); outBuffer += tmp; itor += static_cast( counterVar.size() + 1u ); } @@ -1588,7 +1588,7 @@ namespace Ogre //@value & @counter write, the others are invisible char tmp[16]; - sprintf( tmp, "%i", op1Value ); + std::snprintf( tmp, sizeof( tmp ), "%i", op1Value ); outBuffer += tmp; if( keyword == 0 ) diff --git a/OgreMain/src/OgrePass.cpp b/OgreMain/src/OgrePass.cpp index dde4acc27f4..8a1263cf03f 100644 --- a/OgreMain/src/OgrePass.cpp +++ b/OgreMain/src/OgrePass.cpp @@ -433,9 +433,8 @@ namespace Ogre // allow 8 digit hex number. there should never be that many texture units. // This sprintf replaced a call to StringConverter::toString for performance reasons - char buff[9]; - memset( buff, 0, 9 ); - sprintf( buff, "%lx", static_cast( idx ) ); + char buff[9] = {}; + std::snprintf( buff, sizeof( buff ), "%lx", static_cast( idx ) ); state->setName( buff ); /** since the name was never set and a default one has been made, clear the alias diff --git a/OgreMain/src/OgreSearchOps.cpp b/OgreMain/src/OgreSearchOps.cpp index 27683ee7b98..f56202f04ea 100644 --- a/OgreMain/src/OgreSearchOps.cpp +++ b/OgreMain/src/OgreSearchOps.cpp @@ -30,10 +30,10 @@ THE SOFTWARE. #include "OgreSearchOps.h" #include #include -#include #include #include #include +#include // std::snprintf /* Win32 directory operations emulation */ #if OGRE_PLATFORM != OGRE_PLATFORM_WIN32 && OGRE_PLATFORM != OGRE_PLATFORM_WINRT @@ -113,8 +113,9 @@ int _findnext( intptr_t id, struct _finddata_t *data ) data->name = fs->curfn = strdup( entry->d_name ); size_t namelen = strlen( entry->d_name ); - char *xfn = new char[static_cast( fs->dirlen ) + 1 + namelen + 1]; - sprintf( xfn, "%s/%s", fs->directory, entry->d_name ); + const size_t xfnlen = static_cast( fs->dirlen ) + 1 + namelen + 1; + char *xfn = new char[xfnlen]; + std::snprintf( xfn, xfnlen, "%s/%s", fs->directory, entry->d_name ); /* stat the file to get if it's a subdir and to find its length */ struct stat stat_buf;