Always return -1 but set errno in our wxVsnprintf() on error
This makes it more compatible with the standard behaviour of vswprintf() and allows to use the same logic in the builds using our version of this function and the normal ones in DoStringPrintfV(), simplifying its (already rather hairy) logic. Update the tests to not require any particular return value in case of buffer overflow, as this differs between Unicode and non-Unicode builds. When we finally drop the latter, we should just check that it always returns -1 in this case. Note that ideal would be to return the actually needed size of the buffer in case of the error due to buffer being too small, but this isn't that simple to do and it's probably not worth spending time on improving this code as long as we still need to use the buffer doubling strategy in DoStringPrintfV() when using the standard vswprintf().
This commit is contained in:
@@ -2060,17 +2060,6 @@ static int DoStringPrintfV(wxString& str,
|
|||||||
// buffer were large enough (newer standards such as Unix98)
|
// buffer were large enough (newer standards such as Unix98)
|
||||||
if ( len < 0 )
|
if ( len < 0 )
|
||||||
{
|
{
|
||||||
// NB: wxVsnprintf() may call either wxCRT_VsnprintfW or
|
|
||||||
// wxCRT_VsnprintfA in UTF-8 build; wxUSE_WXVSNPRINTF
|
|
||||||
// is true if *both* of them use our own implementation,
|
|
||||||
// otherwise we can't be sure
|
|
||||||
#if wxUSE_WXVSNPRINTF
|
|
||||||
// we know that our own implementation of wxVsnprintf() returns -1
|
|
||||||
// only for a format error - thus there's something wrong with
|
|
||||||
// the user's format string
|
|
||||||
buf[0] = '\0';
|
|
||||||
return -1;
|
|
||||||
#else // possibly using system version
|
|
||||||
// assume it only returns error if there is not enough space, but
|
// assume it only returns error if there is not enough space, but
|
||||||
// as we don't know how much we need, double the current size of
|
// as we don't know how much we need, double the current size of
|
||||||
// the buffer
|
// the buffer
|
||||||
@@ -2082,16 +2071,9 @@ static int DoStringPrintfV(wxString& str,
|
|||||||
// still not enough, as we don't know how much we need, double the
|
// still not enough, as we don't know how much we need, double the
|
||||||
// current size of the buffer
|
// current size of the buffer
|
||||||
size *= 2;
|
size *= 2;
|
||||||
#endif // wxUSE_WXVSNPRINTF/!wxUSE_WXVSNPRINTF
|
|
||||||
}
|
}
|
||||||
else if ( len >= size )
|
else if ( len >= size )
|
||||||
{
|
{
|
||||||
#if wxUSE_WXVSNPRINTF
|
|
||||||
// we know that our own implementation of wxVsnprintf() returns
|
|
||||||
// size+1 when there's not enough space but that's not the size
|
|
||||||
// of the required buffer!
|
|
||||||
size *= 2; // so we just double the current size of the buffer
|
|
||||||
#else
|
|
||||||
// some vsnprintf() implementations NUL-terminate the buffer and
|
// some vsnprintf() implementations NUL-terminate the buffer and
|
||||||
// some don't in len == size case, to be safe always add 1
|
// some don't in len == size case, to be safe always add 1
|
||||||
// FIXME: I don't quite understand this comment. The vsnprintf
|
// FIXME: I don't quite understand this comment. The vsnprintf
|
||||||
@@ -2100,7 +2082,6 @@ static int DoStringPrintfV(wxString& str,
|
|||||||
// So OF COURSE you need to add 1 to get the right buffer size.
|
// So OF COURSE you need to add 1 to get the right buffer size.
|
||||||
// The following line is definitely correct, no question.
|
// The following line is definitely correct, no question.
|
||||||
size = len + 1;
|
size = len + 1;
|
||||||
#endif
|
|
||||||
}
|
}
|
||||||
else // ok, there was enough space
|
else // ok, there was enough space
|
||||||
{
|
{
|
||||||
|
@@ -25,6 +25,7 @@
|
|||||||
|
|
||||||
#include "wx/private/wxprintf.h"
|
#include "wx/private/wxprintf.h"
|
||||||
|
|
||||||
|
#include <errno.h>
|
||||||
|
|
||||||
// ============================================================================
|
// ============================================================================
|
||||||
// printf() implementation
|
// printf() implementation
|
||||||
@@ -106,6 +107,9 @@ static int wxDoVsnprintf(CharType *buf, size_t lenMax,
|
|||||||
if (parser.posarg_present && parser.nonposarg_present)
|
if (parser.posarg_present && parser.nonposarg_present)
|
||||||
{
|
{
|
||||||
buf[0] = 0;
|
buf[0] = 0;
|
||||||
|
// Indicate to the caller that it's an unrecoverable error and not just
|
||||||
|
// due to the buffer being too small.
|
||||||
|
errno = EINVAL;
|
||||||
return -1; // format strings with both positional and
|
return -1; // format strings with both positional and
|
||||||
} // non-positional conversion specifier are unsupported !!
|
} // non-positional conversion specifier are unsupported !!
|
||||||
|
|
||||||
@@ -131,6 +135,7 @@ static int wxDoVsnprintf(CharType *buf, size_t lenMax,
|
|||||||
if (!ok)
|
if (!ok)
|
||||||
{
|
{
|
||||||
buf[0] = 0;
|
buf[0] = 0;
|
||||||
|
errno = EINVAL;
|
||||||
return -1;
|
return -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -154,7 +159,7 @@ static int wxDoVsnprintf(CharType *buf, size_t lenMax,
|
|||||||
if (lenCur == lenMax)
|
if (lenCur == lenMax)
|
||||||
{
|
{
|
||||||
buf[lenMax - 1] = 0;
|
buf[lenMax - 1] = 0;
|
||||||
return lenMax+1; // not enough space in the output buffer !
|
return -1; // not enough space in the output buffer !
|
||||||
}
|
}
|
||||||
|
|
||||||
// process this specifier directly in the output buffer
|
// process this specifier directly in the output buffer
|
||||||
@@ -163,7 +168,7 @@ static int wxDoVsnprintf(CharType *buf, size_t lenMax,
|
|||||||
if (n == -1)
|
if (n == -1)
|
||||||
{
|
{
|
||||||
buf[lenMax-1] = wxT('\0'); // be sure to always NUL-terminate the string
|
buf[lenMax-1] = wxT('\0'); // be sure to always NUL-terminate the string
|
||||||
return lenMax+1; // not enough space in the output buffer !
|
return -1; // not enough space in the output buffer !
|
||||||
}
|
}
|
||||||
lenCur += n;
|
lenCur += n;
|
||||||
|
|
||||||
@@ -183,7 +188,7 @@ static int wxDoVsnprintf(CharType *buf, size_t lenMax,
|
|||||||
if (buf[lenCur])
|
if (buf[lenCur])
|
||||||
{
|
{
|
||||||
buf[lenCur] = 0;
|
buf[lenCur] = 0;
|
||||||
return lenMax+1; // not enough space in the output buffer !
|
return -1; // not enough space in the output buffer !
|
||||||
}
|
}
|
||||||
|
|
||||||
// Don't do:
|
// Don't do:
|
||||||
|
@@ -52,11 +52,10 @@ int r;
|
|||||||
// Another helper which takes the size explicitly instead of using MAX_TEST_LEN
|
// Another helper which takes the size explicitly instead of using MAX_TEST_LEN
|
||||||
//
|
//
|
||||||
// NOTE: this macro is used also with too-small buffers (see Miscellaneous())
|
// NOTE: this macro is used also with too-small buffers (see Miscellaneous())
|
||||||
// test function, thus the return value can be > size and thus we
|
// test function, thus the return value can be either -1 or > size and we
|
||||||
// cannot check if r == (int)wxStrlen(buf)
|
// cannot check if r == (int)wxStrlen(buf)
|
||||||
#define CMPTOSIZE(buffer, size, failuremsg, expected, fmt, ...) \
|
#define CMPTOSIZE(buffer, size, failuremsg, expected, fmt, ...) \
|
||||||
r=wxSnprintf(buffer, size, fmt, ##__VA_ARGS__); \
|
r=wxSnprintf(buffer, size, fmt, ##__VA_ARGS__); \
|
||||||
CHECK( r > 0 ); \
|
|
||||||
INFO(failuremsg); \
|
INFO(failuremsg); \
|
||||||
CHECK( buffer == wxString(expected).Left(size - 1) )
|
CHECK( buffer == wxString(expected).Left(size - 1) )
|
||||||
|
|
||||||
@@ -340,8 +339,10 @@ TEST_CASE_METHOD(VsnprintfTestCase, "Vsnprintf::WrongFormatStrings", "[vsnprintf
|
|||||||
wxSnprintf(buf, MAX_TEST_LEN, wxT("%1$d %3$d"), 1, 2, 3) );
|
wxSnprintf(buf, MAX_TEST_LEN, wxT("%1$d %3$d"), 1, 2, 3) );
|
||||||
|
|
||||||
// positional and non-positionals in the same format string:
|
// positional and non-positionals in the same format string:
|
||||||
|
errno = 0;
|
||||||
r = wxSnprintf(buf, MAX_TEST_LEN, wxT("%1$d %d %3$d"), 1, 2, 3);
|
r = wxSnprintf(buf, MAX_TEST_LEN, wxT("%1$d %d %3$d"), 1, 2, 3);
|
||||||
CHECK( r == -1 );
|
CHECK( r == -1 );
|
||||||
|
CHECK( errno == EINVAL );
|
||||||
}
|
}
|
||||||
|
|
||||||
// BigToSmallBuffer() test case helper:
|
// BigToSmallBuffer() test case helper:
|
||||||
@@ -392,7 +393,6 @@ void VsnprintfTestCase::DoBigToSmallBuffer(T *buffer, int size)
|
|||||||
wxString expected =
|
wxString expected =
|
||||||
wxString(wxT("unicode string/char: unicode/U -- ansi string/char: ansi/A")).Left(size - 1);
|
wxString(wxT("unicode string/char: unicode/U -- ansi string/char: ansi/A")).Left(size - 1);
|
||||||
|
|
||||||
CHECK( r != -1 );
|
|
||||||
CHECK( expected == buffer );
|
CHECK( expected == buffer );
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Reference in New Issue
Block a user