Skip to content

Commit

Permalink
Use std::snprintf() rather than sprintf(). Use specifically std versi…
Browse files Browse the repository at this point in the history 
…on to catch #define snprintf _snprintf during compile time, as we want standard always NULL terminating behavior. Fixed asserts limits in OgreLwString.h
  • Loading branch information
@eugenegff
eugenegff committed Aug 27, 2024
1 parent 17ef177 commit b375976
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 133 deletions.
62 changes: 9 additions & 53 deletions OgreMain/include/OgreIdString.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ THE SOFTWARE.
#include "OgrePrerequisites.h"

#include <inttypes.h> // PRIx64
#include <stdio.h> // sprintf
#include <string.h> // strlen
#include <cstdio> // std::snprintf
#include <string>

#include "Hash/MurmurHash3.h"
Expand Down Expand Up @@ -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 )
{
Expand Down Expand Up @@ -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 )
Expand Down Expand Up @@ -217,9 +212,6 @@ namespace Ogre
}
}

# if OGRE_COMPILER == OGRE_COMPILER_MSVC
# pragma warning( pop )
# endif
#endif

void operator+=( IdString idString )
Expand Down Expand Up @@ -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.
Expand All @@ -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
}

Expand Down
11 changes: 1 addition & 10 deletions OgreMain/include/OgreLwConstString.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,11 @@
#include <assert.h>
#include <string.h>
#include <algorithm>
#include <cstdio>

#if !defined( _MSC_VER ) || ( _MSC_VER > 1600 )
# include <stdint.h>
#endif
#include <stdio.h>

#ifdef _MSC_VER
# pragma warning( push ) // CRT deprecation
# pragma warning( disable : 4996 )
#endif

namespace Ogre
{
Expand Down Expand Up @@ -179,8 +174,4 @@ namespace Ogre
};
} // namespace Ogre

#ifdef _MSC_VER
# pragma warning( pop )
#endif

#endif
87 changes: 34 additions & 53 deletions OgreMain/include/OgreLwString.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,6 @@
# include <inttypes.h>
#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.
Expand All @@ -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).
Expand Down Expand Up @@ -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<size_t>( 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<size_t>( 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<size_t>( 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<size_t>( 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<size_t>( 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<size_t>( 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<size_t>( 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<size_t>( mSize + (size_t)std::max( required, 0 ), mCapacity - 1u );
return *this;
}

Expand Down Expand Up @@ -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<size_t>( mSize + (size_t)std::max( written, 0 ), mCapacity - 1u );
assert( ( required >= 0 ) && ( (unsigned)required < mCapacity - mSize ) );
mSize = std::min<size_t>( mSize + (size_t)std::max( required, 0 ), mCapacity - 1u );
return *this;
}

Expand Down Expand Up @@ -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<size_t>( mSize + static_cast<size_t>( std::max( written, 0 ) ),
assert( ( required >= 0 ) && ( (unsigned)required < mCapacity - mSize ) );
mSize = std::min<size_t>( mSize + static_cast<size_t>( std::max( required, 0 ) ),
static_cast<size_t>( mCapacity - 1u ) );
return *this;
}
Expand Down Expand Up @@ -493,10 +480,4 @@ namespace Ogre
};
} // namespace Ogre

#ifdef OGRE_LWSTRING_SNPRINTF_DEFINED
# undef _snprintf
#else
# pragma warning( pop )
#endif

#endif
18 changes: 9 additions & 9 deletions OgreMain/src/OgreAssert.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
#include "OgreAssert.h"

#include <stdarg.h>
#include <stdio.h>
#include <cstdio> // std::snprintf

namespace Ogre
{
Expand All @@ -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 ) );

Expand Down
4 changes: 2 additions & 2 deletions OgreMain/src/OgreHlms.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<ptrdiff_t>( counterVar.size() + 1u );
}
Expand Down Expand Up @@ -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 )
Expand Down
5 changes: 2 additions & 3 deletions OgreMain/src/OgrePass.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<long>( idx ) );
char buff[9] = {};
std::snprintf( buff, sizeof( buff ), "%lx", static_cast<long>( idx ) );
state->setName( buff );

/** since the name was never set and a default one has been made, clear the alias
Expand Down
Loading

0 comments on commit b375976

Please sign in to comment.