SDL_error: simplified error string management.

This patch removes deferred error string formatting: now we do it during
SDL_SetError(), so there's no limit on printf-style arguments used.

Also removes stub for managing error string translations; we don't have the
facilities to maintain that and the way we set arbitrary error strings
doesn't really make this practical anyhow.

Since the final error string is set right away and unique to the thread,
we no longer need a static buffer for legacy SDL_GetError(), and we don't
have to allocate 5x 128-byte argument fields per-thread. Also, since we now
use SDL_vsnprintf instead of parsing the format string ourselves, there's a
lot of code deleted and we have access to more robust formatting powers now.

This does mean the final error strings can't be more than 128 bytes, down
from the theoretical maximum of around 768, but I think this is probably okay.
They might truncate but they will always be null-terminated!

Fixes Bugzilla #5092.
This commit is contained in:
Ryan C. Gordon 2020-04-21 01:30:36 -04:00
parent 67760f0ed7
commit 09ca66bf66
2 changed files with 22 additions and 231 deletions

View File

@ -27,98 +27,26 @@
#define SDL_ERRBUFIZE 1024 #define SDL_ERRBUFIZE 1024
/* Private functions */
static const char *
SDL_LookupString(const char *key)
{
/* FIXME: Add code to lookup key in language string hash-table */
return key;
}
/* Public functions */
int int
SDL_SetError(SDL_PRINTF_FORMAT_STRING const char *fmt, ...) SDL_SetError(SDL_PRINTF_FORMAT_STRING const char *fmt, ...)
{ {
va_list ap;
SDL_error *error;
/* Ignore call if invalid format pointer was passed */ /* Ignore call if invalid format pointer was passed */
if (fmt == NULL) return -1; if (fmt != NULL) {
va_list ap;
SDL_error *error = SDL_GetErrBuf();
/* Copy in the key, mark error as valid */ error->error = 1; /* mark error as valid */
error = SDL_GetErrBuf();
error->error = 1;
SDL_strlcpy((char *) error->key, fmt, sizeof(error->key));
va_start(ap, fmt); va_start(ap, fmt);
error->argc = 0; SDL_vsnprintf(error->str, ERR_MAX_STRLEN, fmt, ap);
while (*fmt) {
if (*fmt++ == '%') {
while (*fmt == '.' || (*fmt >= '0' && *fmt <= '9')) {
++fmt;
}
switch (*fmt++) {
case 0: /* Malformed format string.. */
--fmt;
break;
case 'l':
switch (*fmt++) {
case 0: /* Malformed format string.. */
--fmt;
break;
case 'i': case 'd': case 'u': case 'x': case 'X':
error->args[error->argc++].value_l = va_arg(ap, long);
break;
}
break;
case 'c':
case 'i':
case 'd':
case 'u':
case 'o':
case 'x':
case 'X':
error->args[error->argc++].value_i = va_arg(ap, int);
break;
case 'f':
error->args[error->argc++].value_f = va_arg(ap, double);
break;
case 'p':
error->args[error->argc++].value_ptr = va_arg(ap, void *);
break;
case 's':
{
int i = error->argc;
const char *str = va_arg(ap, const char *);
if (str == NULL)
str = "(null)";
SDL_strlcpy((char *) error->args[i].buf, str,
ERR_MAX_STRLEN);
error->argc++;
}
break;
default:
break;
}
if (error->argc >= ERR_MAX_ARGS) {
break;
}
}
}
va_end(ap); va_end(ap);
if (SDL_LogGetPriority(SDL_LOG_CATEGORY_ERROR) <= SDL_LOG_PRIORITY_DEBUG) { if (SDL_LogGetPriority(SDL_LOG_CATEGORY_ERROR) <= SDL_LOG_PRIORITY_DEBUG) {
/* If we are in debug mode, print out an error message /* If we are in debug mode, print out the error message */
* Avoid stomping on the static buffer in GetError, just SDL_LogDebug(SDL_LOG_CATEGORY_ERROR, "%s", error->str);
* in case this is called while processing a ShowMessageBox to
* show an error already in that static buffer.
*/
char errmsg[SDL_ERRBUFIZE];
SDL_GetErrorMsg(errmsg, sizeof(errmsg));
SDL_LogDebug(SDL_LOG_CATEGORY_ERROR, "%s", errmsg);
} }
}
return -1; return -1;
} }
@ -126,18 +54,14 @@ SDL_SetError(SDL_PRINTF_FORMAT_STRING const char *fmt, ...)
const char * const char *
SDL_GetError(void) SDL_GetError(void)
{ {
static char errmsg[SDL_ERRBUFIZE]; const SDL_error *error = SDL_GetErrBuf();
return error->error ? error->str : "";
return SDL_GetErrorMsg(errmsg, SDL_ERRBUFIZE);
} }
void void
SDL_ClearError(void) SDL_ClearError(void)
{ {
SDL_error *error; SDL_GetErrBuf()->error = 0;
error = SDL_GetErrBuf();
error->error = 0;
} }
/* Very common errors go here */ /* Very common errors go here */
@ -178,129 +102,18 @@ main(int argc, char *argv[])
#endif #endif
/* keep this at the end of the file so it works with GCC builds that don't
support "#pragma GCC diagnostic push" ... we'll just leave the warning
disabled after this. */
/* this pragma arrived in GCC 4.2 and causes a warning on older GCCs! Sigh. */
#if defined(__clang__) || (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4 && (__GNUC_MINOR__ >= 2))))
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
#endif
/* This function has a bit more overhead than most error functions
so that it supports internationalization and thread-safe errors.
*/
char * char *
SDL_GetErrorMsg(char *errstr, int maxlen) SDL_GetErrorMsg(char *errstr, int maxlen)
{ {
SDL_error *error; const SDL_error *error = SDL_GetErrBuf();
/* Clear the error string */
*errstr = '\0';
--maxlen;
/* Get the thread-safe error, and print it out */
error = SDL_GetErrBuf();
if (error->error) { if (error->error) {
const char *fmt; SDL_strlcpy(errstr, error->str, maxlen);
char *msg = errstr;
int len;
int argi;
fmt = SDL_LookupString(error->key);
argi = 0;
while (*fmt && (maxlen > 0)) {
if (*fmt == '%') {
char tmp[32], *spot = tmp;
*spot++ = *fmt++;
while ((*fmt == '.' || (*fmt >= '0' && *fmt <= '9'))
&& spot < (tmp + SDL_arraysize(tmp) - 2)) {
*spot++ = *fmt++;
}
if (*fmt == 'l') {
*spot++ = *fmt++;
*spot++ = *fmt++;
*spot++ = '\0';
switch (spot[-2]) {
case 'i': case 'd': case 'u': case 'x': case 'X':
len = SDL_snprintf(msg, maxlen, tmp,
error->args[argi++].value_l);
if (len > 0) {
msg += len;
maxlen -= len;
}
break;
}
continue;
}
*spot++ = *fmt++;
*spot++ = '\0';
switch (spot[-2]) {
case '%':
*msg++ = '%';
maxlen -= 1;
break;
case 'c':
case 'i':
case 'd':
case 'u':
case 'o':
case 'x':
case 'X':
len =
SDL_snprintf(msg, maxlen, tmp,
error->args[argi++].value_i);
if (len > 0) {
msg += len;
maxlen -= len;
}
break;
case 'f':
len =
SDL_snprintf(msg, maxlen, tmp,
error->args[argi++].value_f);
if (len > 0) {
msg += len;
maxlen -= len;
}
break;
case 'p':
len =
SDL_snprintf(msg, maxlen, tmp,
error->args[argi++].value_ptr);
if (len > 0) {
msg += len;
maxlen -= len;
}
break;
case 's':
len =
SDL_snprintf(msg, maxlen, tmp,
SDL_LookupString(error->args[argi++].
buf));
if (len > 0) {
msg += len;
maxlen -= len;
}
break;
}
} else { } else {
*msg++ = *fmt++; *errstr = '\0';
maxlen -= 1;
}
} }
/* slide back if we've overshot the end of our buffer. */ return errstr;
if (maxlen < 0) {
msg -= (-maxlen) + 1;
}
*msg = 0; /* NULL terminate the string */
}
return (errstr);
} }
/* vi: set ts=4 sw=4 expandtab: */ /* vi: set ts=4 sw=4 expandtab: */

View File

@ -28,33 +28,11 @@
#define SDL_error_c_h_ #define SDL_error_c_h_
#define ERR_MAX_STRLEN 128 #define ERR_MAX_STRLEN 128
#define ERR_MAX_ARGS 5
typedef struct SDL_error typedef struct SDL_error
{ {
/* This is a numeric value corresponding to the current error */ int error; /* This is a numeric value corresponding to the current error */
int error; char str[ERR_MAX_STRLEN];
/* This is a key used to index into a language hashtable containing
internationalized versions of the SDL error messages. If the key
is not in the hashtable, or no hashtable is available, the key is
used directly as an error message format string.
*/
char key[ERR_MAX_STRLEN];
/* These are the arguments for the error functions */
int argc;
union
{
void *value_ptr;
#if 0 /* What is a character anyway? (UNICODE issues) */
unsigned char value_c;
#endif
int value_i;
long value_l;
double value_f;
char buf[ERR_MAX_STRLEN];
} args[ERR_MAX_ARGS];
} SDL_error; } SDL_error;
/* Defined in SDL_thread.c */ /* Defined in SDL_thread.c */