wxSnprintf() and wxVsnprintf() added, documented and used in wxLog

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@4572 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

docs/latex/wx/function.tex

@@ -412,6 +412,33 @@ should help to find the strings which were not yet translated. As this function
 is used very often, an alternative syntax is provided: the \_() macro is
 defined as wxGetTranslation().
 
+\membersection{::wxSnprintf}\label{wxsnprintf}
+
+\func{int}{wxSnprintf}{\param{wxChar *}{buf}, \param{size\_t }{len}, \param{const wxChar *}{format}, \param{}{...}}
+
+This function replaces the dangerous standard function {\tt sprintf()} and is
+like {\tt snprintf()} available on some platforms. The only difference with
+sprintf() is that an additional argument - buffer size - is taken and the
+buffer is never overflowed.
+
+Returns the number of characters copied to the buffer or -1 if there is not
+enough space.
+
+\wxheading{See also:}
+\helpref{wxVsnprintf}{wxvsnprintf},
+\helpref{wxString::Printf}{wxstringprintf}
+
+\membersection{::wxVsnprintf}\label{wxsnprintf}
+
+\func{int}{wxVsnprintf}{\param{wxChar *}{buf}, \param{size\_t }{len}, \param{const wxChar *}{format}, \param{va\_list }{argptr}}
+
+The same as \helpref{wxSnprintf}{wxsnprintf} but takes a {\tt va\_list}
+argument instead of arbitrary number of parameters.
+
+\wxheading{See also:}
+\helpref{wxSnprintf}{wxsnprintf},
+\helpref{wxString::PrintfV}{wxstringprintfv}
+
 \section{Dialog functions}\label{dialogfunctions}
 
 Below are a number of convenience functions for getting input from the

docs/latex/wx/tstring.tex

@@ -144,6 +144,13 @@ and returns 0 for them and \helpref{Stricmp()}{Stricmp} is just a
 platform-independent version of case-insensitive string comparison function
 known either as stricmp() or strcasecmp() on different platforms.
 
+The {\tt <wx/string.h>} header also defines \helpref{wxSnprintf}{wxsnprintf} 
+and \helpref{wxVsnprintf}{wxvsnprintf} functions which should be used instead
+of the inherently dangerous standard {\tt sprintf()} and which use {\tt
+snprintf()} instead which does buffer size checks whenever possible. Of
+course, you may also use \helpref{wxString::Printf}{wxstringprintf} which is
+also safe.
+
 There is another class which might be useful when working with wxString: 
 \helpref{wxStringTokenizer}{wxstringtokenizer}. It is helpful when a string must
 be broken into tokens and replaces the standard C library {\it

include/wx/string.h

@@ -156,6 +156,15 @@ inline int WXDLLEXPORT Stricmp(const char *psz1, const char *psz2)
 #endif  // OS/compiler
 }
 
+// wxSnprintf() is like snprintf() if it's available and sprintf() (always
+// available, but dangerous!) if not
+extern int WXDLLEXPORT wxSnprintf(wxChar *buf, size_t len,
+                                  const wxChar *format, ...);
+
+// and wxVsnprintf() is like vsnprintf() or vsprintf()
+extern int WXDLLEXPORT wxVsnprintf(wxChar *buf, size_t len,
+                                   const wxChar *format, va_list argptr);
+
 // return an empty wxString
 class WXDLLEXPORT wxString; // not yet defined
 inline const wxString& wxGetEmptyString() { return *(wxString *)&wxEmptyString; }

samples/console/console.cpp

@@ -28,8 +28,10 @@
 // ----------------------------------------------------------------------------
 
 // what to test?
-#define TEST_ARRAYS
+
-#undef TEST_THREADS
+//#define TEST_ARRAYS
+#define TEST_LOG
+//#define TEST_THREADS
 
 // ============================================================================
 // implementation
@@ -162,6 +164,23 @@ int main(int argc, char **argv)
     PrintArray("a3", a3);
 #endif // TEST_ARRAYS
 
+#ifdef TEST_LOG
+    wxString s;
+    for ( size_t n = 0; n < 8000; n++ )
+    {
+        s << (char)('A' + (n % 26));
+    }
+
+    wxString msg;
+    msg.Printf("A very very long message: '%s', the end!\n", s.c_str());
+
+    // this one shouldn't be truncated
+    printf(msg);
+
+    // but this one will because log functions use fixed size buffer
+    wxLogMessage("A very very long message 2: '%s', the end!\n", s.c_str());
+#endif // TEST_LOG
+
 #ifdef TEST_THREADS
     static const size_t nThreads = 3;
     MyThread *threads[nThreads];

src/common/log.cpp

@@ -97,7 +97,7 @@ void wxLogGeneric(wxLogLevel level, const wxChar *szFormat, ...)
   if ( wxLog::GetActiveTarget() != NULL ) {
     va_list argptr;
     va_start(argptr, szFormat);
-    wxVsprintf(s_szBuf, szFormat, argptr);
+    wxVsnprintf(s_szBuf, WXSIZEOF(s_szBuf), szFormat, argptr);
     va_end(argptr);
 
     wxLog::OnLog(level, s_szBuf, time(NULL));
@@ -110,7 +110,7 @@ void wxLogGeneric(wxLogLevel level, const wxChar *szFormat, ...)
     if ( wxLog::GetActiveTarget() != NULL ) {                     \
       va_list argptr;                                             \
       va_start(argptr, szFormat);                                 \
-      wxVsprintf(s_szBuf, szFormat, argptr);                      \
+      wxVsnprintf(s_szBuf, WXSIZEOF(s_szBuf), szFormat, argptr);  \
       va_end(argptr);                                             \
                                                                   \
       wxLog::OnLog(wxLOG_##level, s_szBuf, time(NULL));           \
@@ -131,7 +131,7 @@ void wxLogVerbose(const wxChar *szFormat, ...)
   if ( pLog != NULL && pLog->GetVerbose() ) {
     va_list argptr;
     va_start(argptr, szFormat);
-    wxVsprintf(s_szBuf, szFormat, argptr);
+    wxVsnprintf(s_szBuf, WXSIZEOF(s_szBuf), szFormat, argptr);
     va_end(argptr);
 
     wxLog::OnLog(wxLOG_Info, s_szBuf, time(NULL));
@@ -146,7 +146,7 @@ void wxLogVerbose(const wxChar *szFormat, ...)
     if ( wxLog::GetActiveTarget() != NULL ) {                     \
       va_list argptr;                                             \
       va_start(argptr, szFormat);                                 \
-      wxVsprintf(s_szBuf, szFormat, argptr);                      \
+      wxVsnprintf(s_szBuf, WXSIZEOF(s_szBuf), szFormat, argptr);  \
       va_end(argptr);                                             \
                                                                   \
       wxLog::OnLog(wxLOG_##level, s_szBuf, time(NULL));           \
@@ -160,7 +160,7 @@ void wxLogVerbose(const wxChar *szFormat, ...)
     if ( pLog != NULL && wxLog::IsAllowedTraceMask(mask) ) {
       va_list argptr;
       va_start(argptr, szFormat);
-      wxVsprintf(s_szBuf, szFormat, argptr);
+      wxVsnprintf(s_szBuf, WXSIZEOF(s_szBuf), szFormat, argptr);
       va_end(argptr);
 
       wxLog::OnLog(wxLOG_Trace, s_szBuf, time(NULL));
@@ -177,7 +177,7 @@ void wxLogVerbose(const wxChar *szFormat, ...)
     if ( pLog != NULL && ((pLog->GetTraceMask() & mask) == mask) ) {
       va_list argptr;
       va_start(argptr, szFormat);
-      wxVsprintf(s_szBuf, szFormat, argptr);
+      wxVsnprintf(s_szBuf, WXSIZEOF(s_szBuf), szFormat, argptr);
       va_end(argptr);
 
       wxLog::OnLog(wxLOG_Trace, s_szBuf, time(NULL));
@@ -198,7 +198,8 @@ IMPLEMENT_LOG_DEBUG_FUNCTION(Trace)
 void wxLogSysErrorHelper(long lErrCode)
 {
     wxChar szErrMsg[LOG_BUFFER_SIZE / 2];
-    wxSprintf(szErrMsg, _(" (error %ld: %s)"), lErrCode, wxSysErrorMsg(lErrCode));
+    wxSnprintf(szErrMsg, WXSIZEOF(szErrMsg),
+               _(" (error %ld: %s)"), lErrCode, wxSysErrorMsg(lErrCode));
     wxStrncat(s_szBuf, szErrMsg, WXSIZEOF(s_szBuf) - wxStrlen(s_szBuf));
 
     wxLog::OnLog(wxLOG_Error, s_szBuf, time(NULL));
@@ -208,7 +209,7 @@ void WXDLLEXPORT wxLogSysError(const wxChar *szFormat, ...)
 {
     va_list argptr;
     va_start(argptr, szFormat);
-    wxVsprintf(s_szBuf, szFormat, argptr);
+    wxVsnprintf(s_szBuf, WXSIZEOF(s_szBuf), szFormat, argptr);
     va_end(argptr);
 
     wxLogSysErrorHelper(wxSysErrorCode());
@@ -218,7 +219,7 @@ void WXDLLEXPORT wxLogSysError(long lErrCode, const wxChar *szFormat, ...)
 {
     va_list argptr;
     va_start(argptr, szFormat);
-    wxVsprintf(s_szBuf, szFormat, argptr);
+    wxVsnprintf(s_szBuf, WXSIZEOF(s_szBuf), szFormat, argptr);
     va_end(argptr);
 
     wxLogSysErrorHelper(lErrCode);
@@ -365,7 +366,7 @@ void wxLogStderr::DoLogString(const wxChar *szString, time_t WXUNUSED(t))
     fputs(str.mb_str(), m_fp);
     fflush(m_fp);
 
-    // under Windows, programs usually don't have stderr at all, so make show the
+    // under Windows, programs usually don't have stderr at all, so show the
     // messages also under debugger
 #ifdef __WXMSW__
     OutputDebugString(str + wxT('\r'));
@@ -558,12 +559,14 @@ void wxOnAssert(const wxChar *szFile, int nLine, const wxChar *szMsg)
 
     // make life easier for people using VC++ IDE: clicking on the message
     // will take us immediately to the place of the failed assert
+    wxSnprintf(szBuf, WXSIZEOF(szBuf),
 #ifdef __VISUALC__
-    wxSprintf(szBuf, wxT("%s(%d): assert failed"), szFile, nLine);
+               wxT("%s(%d): assert failed"),
 #else  // !VC++
     // make the error message more clear for all the others
-    wxSprintf(szBuf, wxT("Assert failed in file %s at line %d"), szFile, nLine);
+               wxT("Assert failed in file %s at line %d"),
 #endif // VC/!VC
+               szFile, nLine);
 
     if ( szMsg != NULL ) {
         wxStrcat(szBuf, wxT(": "));

src/common/string.cpp

@@ -35,10 +35,8 @@
   #include "wx/defs.h"
   #include "wx/string.h"
   #include "wx/intl.h"
-#if wxUSE_THREADS
   #include "wx/thread.h"
 #endif
-#endif
 
 #include <ctype.h>
 #include <string.h>
@@ -104,28 +102,32 @@ extern const wxChar WXDLLEXPORT *wxEmptyString = &g_strEmpty.dummy;
 // we want to find out if the current platform supports vsnprintf()-like
 // function: for Unix this is done with configure, for Windows we test the
 // compiler explicitly.
+//
+// FIXME currently, this is only for ANSI (!Unicode) strings, so we call this
+//       function wxVsnprintfA (A for ANSI), should also find one for Unicode
+//       strings in Unicode build
 #ifdef __WXMSW__
     #ifdef __VISUALC__
-        #define wxVsnprintf     _vsnprintf
+        #define wxVsnprintfA     _vsnprintf
     #endif
 #else   // !Windows
     #ifdef HAVE_VSNPRINTF
-        #define wxVsnprintf       vsnprintf
+        #define wxVsnprintfA       vsnprintf
     #endif
 #endif  // Windows/!Windows
 
-#ifndef wxVsnprintf
+#ifndef wxVsnprintfA
     // in this case we'll use vsprintf() (which is ANSI and thus should be
     // always available), but it's unsafe because it doesn't check for buffer
     // size - so give a warning
-    #define wxVsnprintf(buffer,len,format,argptr) vsprintf(buffer,format, argptr)
+    #define wxVsnprintfA(buf, len, format, arg) vsprintf(buf, format, arg)
 
     #if defined(__VISUALC__)
         #pragma message("Using sprintf() because no snprintf()-like function defined")
     #elif defined(__GNUG__) && !defined(__UNIX__)
         #warning "Using sprintf() because no snprintf()-like function defined"
     #elif defined(__MWERKS__)
-             #warning "Using sprintf() because no snprintf()-like function defined"
+        #warning "Using sprintf() because no snprintf()-like function defined"
     #endif //compiler
 #endif // no vsnprintf
 
@@ -184,6 +186,37 @@ ostream& operator<<(ostream& os, const wxString& str)
 
 #endif  //std::string compatibility
 
+extern int WXDLLEXPORT wxVsnprintf(wxChar *buf, size_t len,
+                                   const wxChar *format, va_list argptr)
+{
+#if wxUSE_UNICODE
+    // FIXME should use wvsnprintf() or whatever if it's available
+    wxString s;
+    int iLen = s.PrintfV(format, argptr);
+    if ( iLen != -1 )
+    {
+        wxStrncpy(buf, s.c_str(), iLen);
+    }
+
+    return iLen;
+#else // ANSI
+    return wxVsnprintfA(buf, len, format, argptr);
+#endif // Unicode/ANSI
+}
+
+extern int WXDLLEXPORT wxSnprintf(wxChar *buf, size_t len,
+                                  const wxChar *format, ...)
+{
+    va_list argptr;
+    va_start(argptr, format);
+
+    int iLen = wxVsnprintf(buf, len, format, argptr);
+
+    va_end(argptr);
+
+    return iLen;
+}
+
 // ----------------------------------------------------------------------------
 // private classes
 // ----------------------------------------------------------------------------
@@ -988,6 +1021,7 @@ wxString& wxString::operator<<(double d)
 // ---------------------------------------------------------------------------
 // formatted output
 // ---------------------------------------------------------------------------
+
 int wxString::Printf(const wxChar *pszFormat, ...)
 {
   va_list argptr;
@@ -1002,18 +1036,11 @@ int wxString::Printf(const wxChar *pszFormat, ...)
 
 int wxString::PrintfV(const wxChar* pszFormat, va_list argptr)
 {
-  // static buffer to avoid dynamic memory allocation each time
-  char s_szScratch[1024]; // using static buffer causes internal compiler err
-#if 0
-#if wxUSE_THREADS
-  // protect the static buffer
-  static wxCriticalSection critsect;
-  wxCriticalSectionLocker lock(critsect);
-#endif
-#endif
-
 #if wxUSE_EXPERIMENTAL_PRINTF
-// the new implementation
+  // the new implementation
+
+  // buffer to avoid dynamic memory allocation each time for small strings
+  char szScratch[1024];
 
   Reinit();
   for (size_t n = 0; pszFormat[n]; n++)
@@ -1120,30 +1147,30 @@ int wxString::PrintfV(const wxChar* pszFormat, va_list argptr)
           s_szFlags[flagofs] = '\0';
           if (ilen == 0 ) {
             int val = va_arg(argptr, int);
-            ::sprintf(s_szScratch, s_szFlags, val);
+            ::sprintf(szScratch, s_szFlags, val);
           }
           else if (ilen == -1) {
             short int val = va_arg(argptr, short int);
-            ::sprintf(s_szScratch, s_szFlags, val);
+            ::sprintf(szScratch, s_szFlags, val);
           }
           else if (ilen == 1) {
             long int val = va_arg(argptr, long int);
-            ::sprintf(s_szScratch, s_szFlags, val);
+            ::sprintf(szScratch, s_szFlags, val);
           }
           else if (ilen == 2) {
 #if SIZEOF_LONG_LONG
             long long int val = va_arg(argptr, long long int);
-            ::sprintf(s_szScratch, s_szFlags, val);
+            ::sprintf(szScratch, s_szFlags, val);
 #else
             long int val = va_arg(argptr, long int);
-            ::sprintf(s_szScratch, s_szFlags, val);
+            ::sprintf(szScratch, s_szFlags, val);
 #endif
           }
           else if (ilen == 3) {
             size_t val = va_arg(argptr, size_t);
-            ::sprintf(s_szScratch, s_szFlags, val);
+            ::sprintf(szScratch, s_szFlags, val);
           }
-          *this += wxString(s_szScratch);
+          *this += wxString(szScratch);
           done = TRUE;
           break;
         case wxT('e'):
@@ -1156,12 +1183,12 @@ int wxString::PrintfV(const wxChar* pszFormat, va_list argptr)
           s_szFlags[flagofs] = '\0';
           if (ilen == 2) {
             long double val = va_arg(argptr, long double);
-            ::sprintf(s_szScratch, s_szFlags, val);
+            ::sprintf(szScratch, s_szFlags, val);
           } else {
             double val = va_arg(argptr, double);
-            ::sprintf(s_szScratch, s_szFlags, val);
+            ::sprintf(szScratch, s_szFlags, val);
           }
-          *this += wxString(s_szScratch);
+          *this += wxString(szScratch);
           done = TRUE;
           break;
         case wxT('p'):
@@ -1170,8 +1197,8 @@ int wxString::PrintfV(const wxChar* pszFormat, va_list argptr)
             CHECK_PREC
             s_szFlags[flagofs++] = pszFormat[n];
             s_szFlags[flagofs] = '\0';
-            ::sprintf(s_szScratch, s_szFlags, val);
+            ::sprintf(szScratch, s_szFlags, val);
-            *this += wxString(s_szScratch);
+            *this += wxString(szScratch);
             done = TRUE;
           }
           break;
@@ -1245,39 +1272,43 @@ int wxString::PrintfV(const wxChar* pszFormat, va_list argptr)
     } else *this += pszFormat[n];
 
 #else
-  // NB: wxVsnprintf() may return either less than the buffer size or -1 if there
+  // buffer to avoid dynamic memory allocation each time for small strings
-  //     is not enough place depending on implementation
+  char szScratch[1024];
-  int iLen = wxVsnprintf(s_szScratch, WXSIZEOF(s_szScratch), pszFormat, argptr);
+
-  char *buffer;
+  // NB: wxVsnprintf() may return either less than the buffer size or -1 if
-  if ( iLen < (int)WXSIZEOF(s_szScratch) ) {
+  //     there is not enough place depending on implementation
-    buffer = s_szScratch;
+  int iLen = wxVsnprintfA(szScratch, WXSIZEOF(szScratch), pszFormat, argptr);
+  if ( iLen != -1 ) {
+    // the whole string is in szScratch
+    *this = szScratch;
   }
   else {
-      int size = WXSIZEOF(s_szScratch) * 2;
+      bool outOfMemory = FALSE;
-      buffer = (char *)malloc(size);
+      int size = 2*WXSIZEOF(szScratch);
-      while ( buffer != NULL ) {
+      while ( !outOfMemory ) {
-          iLen = wxVsnprintf(buffer, WXSIZEOF(s_szScratch), pszFormat, argptr);
+          char *buf = GetWriteBuf(size);
-          if ( iLen < size ) {
+          if ( buf )
+            iLen = wxVsnprintfA(buf, size, pszFormat, argptr);
+          else
+            outOfMemory = TRUE;
+
+          UngetWriteBuf();
+
+          if ( iLen != -1 ) {
               // ok, there was enough space
               break;
           }
 
           // still not enough, double it again
-          buffer = (char *)realloc(buffer, size *= 2);
+          size *= 2;
       }
 
-      if ( !buffer ) {
+      if ( outOfMemory ) {
           // out of memory
           return -1;
       }
   }
-
+#endif // wxUSE_EXPERIMENTAL_PRINTF/!wxUSE_EXPERIMENTAL_PRINTF
-  wxString s(buffer);
-  *this = s;
-
-  if ( buffer != s_szScratch )
-      free(buffer);
-#endif
 
   return Len();
 }