BLI_dynstr: remove workaround for vsnprintf on WIN32

Based on code-comments it seems vsnprintf didn't return the un-clamped
string length on MS-Windows, this is no longer the case.
BLI_dynstr used allocation doubling in a loop (with a 65536 limit)
in an attempt to allocate sufficient space.

This workaround isn't needed anymore. Expose BLI_sprintfN_with_buffer &
BLI_vsprintfN_with_buffer functions that take a fixed buffer to avoid
allocation for smaller strings.
This simplifies BLI_dynstr_appendf considerably.
This commit is contained in:
Campbell Barton 2023-07-03 17:01:32 +10:00
parent 6fbe467021
commit 1a758aa793
3 changed files with 105 additions and 156 deletions

View File

@ -236,6 +236,19 @@ size_t BLI_vsnprintf_rlen(char *__restrict dst,
const char *__restrict format,
va_list arg) ATTR_PRINTF_FORMAT(3, 0);
char *BLI_sprintfN_with_buffer(char *fixed_buf,
size_t fixed_buf_size,
size_t *result_len,
const char *__restrict format,
...) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1, 3, 4)
ATTR_PRINTF_FORMAT(4, 5);
char *BLI_vsprintfN_with_buffer(char *fixed_buf,
size_t fixed_buf_size,
size_t *result_len,
const char *__restrict format,
va_list args) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1, 3, 4)
ATTR_PRINTF_FORMAT(4, 0);
/**
* Print formatted string into a newly #MEM_mallocN'd string
* and return it.
@ -243,7 +256,7 @@ size_t BLI_vsnprintf_rlen(char *__restrict dst,
char *BLI_sprintfN(const char *__restrict format, ...) ATTR_WARN_UNUSED_RESULT
ATTR_NONNULL(1) ATTR_MALLOC ATTR_PRINTF_FORMAT(1, 2);
/** A version of #BLI_sprintfN that takes a #va_list. */
char *BLI_vsprintfN(const char *__restrict format, va_list args) ATTR_NONNULL(1, 2)
char *BLI_vsprintfN(const char *__restrict format, va_list args) ATTR_NONNULL(1, 2) ATTR_MALLOC
ATTR_PRINTF_FORMAT(1, 0);
/**

View File

@ -17,20 +17,6 @@
#include "BLI_utildefines.h"
#include "MEM_guardedalloc.h"
#ifdef _WIN32
# ifndef vsnprintf
# define vsnprintf _vsnprintf
# endif
#endif
#ifndef va_copy
# ifdef __va_copy
# define va_copy(a, b) __va_copy(a, b)
# else /* !__va_copy */
# define va_copy(a, b) ((a) = (b))
# endif /* __va_copy */
#endif /* va_copy */
/***/
typedef struct DynStrElem DynStrElem;
@ -114,121 +100,27 @@ void BLI_dynstr_nappend(DynStr *__restrict ds, const char *cstr, int len)
void BLI_dynstr_vappendf(DynStr *__restrict ds, const char *__restrict format, va_list args)
{
char *message, fixedmessage[256];
int len = sizeof(fixedmessage);
const int maxlen = 65536;
int retval;
while (1) {
va_list args_cpy;
if (len == sizeof(fixedmessage)) {
message = fixedmessage;
}
else {
message = MEM_callocN(sizeof(char) * len, "BLI_dynstr_appendf");
}
/* can't reuse the same args, so work on a copy */
va_copy(args_cpy, args);
retval = vsnprintf(message, len, format, args_cpy);
va_end(args_cpy);
if (retval == -1) {
/* -1 means not enough space, but on windows it may also mean
* there is a formatting error, so we impose a maximum length */
if (message != fixedmessage) {
MEM_freeN(message);
}
message = NULL;
len *= 2;
if (len > maxlen) {
fprintf(stderr, "BLI_dynstr_append text too long or format error.\n");
break;
}
}
else if (retval >= len) {
/* in C99 the actual length required is returned */
if (message != fixedmessage) {
MEM_freeN(message);
}
message = NULL;
/* retval doesn't include \0 terminator */
len = retval + 1;
}
else {
break;
}
}
if (message) {
BLI_dynstr_append(ds, message);
if (message != fixedmessage) {
MEM_freeN(message);
}
char *str, fixed_buf[256];
size_t str_len;
str = BLI_vsprintfN_with_buffer(fixed_buf, sizeof(fixed_buf), &str_len, format, args);
BLI_dynstr_append(ds, str);
if (str != fixed_buf) {
MEM_freeN(str);
}
}
void BLI_dynstr_appendf(DynStr *__restrict ds, const char *__restrict format, ...)
{
va_list args;
char *message, fixedmessage[256];
int len = sizeof(fixedmessage);
const int maxlen = 65536;
int retval;
/* note that it's tempting to just call BLI_dynstr_vappendf here
* and avoid code duplication, that crashes on some system because
* va_start/va_end have to be called for each vsnprintf call */
while (1) {
if (len == sizeof(fixedmessage)) {
message = fixedmessage;
}
else {
message = MEM_callocN(sizeof(char) * (len), "BLI_dynstr_appendf");
}
va_start(args, format);
retval = vsnprintf(message, len, format, args);
va_end(args);
if (retval == -1) {
/* -1 means not enough space, but on windows it may also mean
* there is a formatting error, so we impose a maximum length */
if (message != fixedmessage) {
MEM_freeN(message);
}
message = NULL;
len *= 2;
if (len > maxlen) {
fprintf(stderr, "BLI_dynstr_append text too long or format error.\n");
break;
}
}
else if (retval >= len) {
/* in C99 the actual length required is returned */
if (message != fixedmessage) {
MEM_freeN(message);
}
message = NULL;
/* retval doesn't include \0 terminator */
len = retval + 1;
}
else {
break;
}
}
if (message) {
BLI_dynstr_append(ds, message);
if (message != fixedmessage) {
MEM_freeN(message);
char *str, fixed_buf[256];
size_t str_len;
va_start(args, format);
str = BLI_vsprintfN_with_buffer(fixed_buf, sizeof(fixed_buf), &str_len, format, args);
va_end(args);
if (LIKELY(str)) {
BLI_dynstr_append(ds, str);
if (str != fixed_buf) {
MEM_freeN(str);
}
}
}

View File

@ -226,56 +226,100 @@ size_t BLI_snprintf_rlen(char *__restrict dst,
return n;
}
char *BLI_sprintfN(const char *__restrict format, ...)
char *BLI_sprintfN_with_buffer(
char *fixed_buf, size_t fixed_buf_size, size_t *result_len, const char *__restrict format, ...)
{
va_list args;
char fixedmessage[256];
va_start(args, format);
int retval = vsnprintf(fixedmessage, sizeof(fixedmessage), format, args);
int retval = vsnprintf(fixed_buf, fixed_buf_size, format, args);
va_end(args);
if (UNLIKELY(retval < 0)) {
/* Return an empty string as there was an error there is no valid output. */
return MEM_callocN(sizeof(char), __func__);
*result_len = 0;
if (UNLIKELY(fixed_buf_size == 0)) {
return MEM_callocN(sizeof(char), __func__);
}
*fixed_buf = '\0';
return fixed_buf;
}
*result_len = (size_t)retval;
if (retval < fixed_buf_size) {
return fixed_buf;
}
/* `retval` doesn't include null terminator. */
const size_t size = (size_t)retval + 1;
char *message = MEM_mallocN(sizeof(char) * size, __func__);
char *result = MEM_mallocN(sizeof(char) * size, __func__);
va_start(args, format);
retval = vsnprintf(result, size, format, args);
va_end(args);
BLI_assert(retval + 1 == size);
return result;
}
if (retval < sizeof(fixedmessage)) {
memcpy(message, fixedmessage, size);
char *BLI_vsprintfN_with_buffer(char *fixed_buf,
size_t fixed_buf_size,
size_t *result_len,
const char *__restrict format,
va_list args)
{
va_list args_copy;
va_copy(args_copy, args);
int retval = vsnprintf(fixed_buf, fixed_buf_size, format, args_copy);
va_end(args_copy);
if (UNLIKELY(retval < 0)) {
/* Return an empty string as there was an error there is no valid output. */
*result_len = 0;
if (UNLIKELY(fixed_buf_size == 0)) {
return MEM_callocN(sizeof(char), __func__);
}
*fixed_buf = '\0';
return fixed_buf;
}
else {
va_start(args, format);
retval = vsnprintf(message, size, format, args);
va_end(args);
BLI_assert(retval + 1 == size);
*result_len = (size_t)retval;
if (retval < fixed_buf_size) {
return fixed_buf;
}
return message;
/* `retval` doesn't include null terminator. */
const size_t size = (size_t)retval + 1;
char *result = MEM_mallocN(sizeof(char) * size, __func__);
retval = vsnprintf(result, size, format, args);
BLI_assert(retval + 1 == size);
return result;
}
char *BLI_sprintfN(const char *__restrict format, ...)
{
char fixed_buf[256];
size_t result_len;
va_list args;
va_start(args, format);
char *result = BLI_vsprintfN_with_buffer(
fixed_buf, sizeof(fixed_buf), &result_len, format, args);
va_end(args);
if (result != fixed_buf) {
return result;
}
size_t size = result_len + 1;
result = MEM_mallocN(sizeof(char) * size, __func__);
memcpy(result, fixed_buf, size);
return result;
}
char *BLI_vsprintfN(const char *__restrict format, va_list args)
{
va_list args_copy;
char fixedmessage[256];
va_copy(args_copy, args);
int retval = vsnprintf(fixedmessage, sizeof(fixedmessage), format, args_copy);
va_end(args_copy);
if (UNLIKELY(retval < 0)) {
/* Return an empty string as there was an error there is no valid output. */
return MEM_callocN(sizeof(char), __func__);
char fixed_buf[256];
size_t result_len;
char *result = BLI_vsprintfN_with_buffer(
fixed_buf, sizeof(fixed_buf), &result_len, format, args);
if (result != fixed_buf) {
return result;
}
/* `retval` doesn't include null terminator. */
const size_t size = (size_t)retval + 1;
char *message = MEM_mallocN(sizeof(char) * size, __func__);
if (retval < sizeof(fixedmessage)) {
memcpy(message, fixedmessage, size);
}
else {
retval = vsnprintf(message, size, format, args);
BLI_assert(retval + 1 == size);
}
return message;
size_t size = result_len + 1;
result = MEM_mallocN(sizeof(char) * size, __func__);
memcpy(result, fixed_buf, size);
return result;
}
/** \} */