Normalized uiprivInternalError() calls to always try to fail gracefully, and make sure errors don't call abort() so we can resume when debugging (and because this isn't really our decision to make...). (The debugger breaks will be removed from release builds when we get to that point.) Finally, refined some TODOs and removed some stale ones (in this case, that were related to the first few things).

This commit is contained in:
Pietro Gagliardi 2020-02-09 13:37:45 -05:00
parent 5952ad368d
commit 83129eeef5
8 changed files with 25 additions and 17 deletions

View File

@ -30,8 +30,10 @@ bool uiprivInitReturnErrorf(uiInitError *err, const char *fmt, ...)
va_start(ap, fmt);
n = uiprivVsnprintf(err->Message, 256, fmt, ap);
va_end(ap);
if (n < 0)
if (n < 0) {
uiprivInternalError("encoding error returning initialization error; this means something is very very wrong with libui itself");
abort(); // TODO handle this scenario more gracefully
}
if (n >= 256) {
// the formatted message is too long; truncate it
err->Message[252] = '.';

View File

@ -127,7 +127,6 @@ void uiprivReportError(const char *prefix, const char *msg, const char *suffix,
[NSException raise:exceptionName
format:@"%s: %s", prefix, msg];
debugBreak();
abort(); // we shouldn't reach here
}
#ifdef __clang__

View File

@ -3,8 +3,7 @@
uiprivApplication *uiprivApp;
// TODO add format string warning detection to all these functions, where available
// TODO also see if we can convert this to a string, or use a known type for status_t instead of assuming it's int(32_t)
// TODO see if we can convert this to a string, or use a known type for status_t instead of assuming it's int(32_t)
#define uiprivInitReturnStatus(err, msg, status) uiprivInitReturnErrorf(err, "%s: %ld", msg, status)
static thread_id mainThread;
@ -52,8 +51,10 @@ void uiprivApplication::MessageReceived(BMessage *msg)
case uiprivMsgQueueMain:
status = msg->FindData("args", B_ANY_TYPE,
&data, &size);
if (status != B_OK)
if (status != B_OK) {
uiprivInternalError("BMessage::FindData() failed in uiprivApplication::MessageReceived() for uiQueueMain(): %ld", status);
return;
}
args = (const struct queueMainArgs *) data;
(*(args->f))(args->data);
return;
@ -72,9 +73,11 @@ void uiprivSysQueueMain(void (*f)(void *data), void *data)
msg = new BMessage(uiprivMsgQueueMain);
status = msg->AddData("args", B_RAW_TYPE,
&args, sizeof (struct queueMainArgs), true, 1);
if (status != B_OK)
// TODO decide if we should just give up in this case like we do with user errors
if (status != B_OK) {
uiprivInternalError("BMessage::AddData() failed in uiQueueMain(): %ld", status);
delete msg;
return;
}
status = uiprivApp->PostMessage(msg);
// msg is copied by PostMessage() so we can delete it here
delete msg;
@ -91,5 +94,4 @@ void uiprivReportError(const char *prefix, const char *msg, const char *suffix,
{
fprintf(stderr, "*** %s: %s. %s\n", prefix, msg, suffix);
debugger("TODO");
abort(); // we shouldn't reach here
}

View File

@ -13,8 +13,10 @@ void *sharedbitsPrefixName(Alloc)(size_t n, const char *what)
void *p;
p = malloc(n);
if (p == NULL)
if (p == NULL) {
sharedbitsPrefixName(InternalError)("memory exhausted allocating %s", what);
abort(); // TODO handle more gracefully somehow
}
memset(p, 0, n);
return p;
}
@ -22,8 +24,10 @@ void *sharedbitsPrefixName(Alloc)(size_t n, const char *what)
void *sharedbitsPrefixName(Realloc)(void *p, size_t nOld, size_t nNew, const char *what)
{
p = realloc(p, nNew);
if (p == NULL)
if (p == NULL) {
sharedbitsPrefixName(InternalError)("memory exhausted reallocating %s", what);
abort(); // TODO handle more gracefully somehow
}
if (nNew > nOld)
memset(((uint8_t *) p) + nOld, 0, nNew - nOld);
return p;

View File

@ -16,7 +16,7 @@ int sharedbitsPrefixName(Vsnprintf)(char *s, size_t n, const char *fmt, va_list
// TODO figure out how to disambiguate between encoding errors (returns negative value; does not have documented errno values), other errors (returns negative value; errno == EINVAL), and truncations (returns -1; does not have documented errno values)
ret = vsnprintf_s(s, n, _TRUNCATE, fmt, ap);
if (ret == -1)
// TODO make this safe
// TODO make this safe (by having these functions return size_t? I forget now...)
return (int) n;
return ret;
#else
@ -52,7 +52,7 @@ char *sharedbitsPrefixName(Strncpy)(char *dest, const char *src, size_t n)
// because strncpy_s() doesn't do this
memset(dest, '\0', n * sizeof (char));
err = strncpy_s(dest, n, src, _TRUNCATE);
if (err != 0 && err != STRUNCATE)
if (err != 0 && err != STRUNCATE) {
// Yes folks, apparently strerror() is unsafe (it's not reentrant, but that's not the point of the MSVC security functions; that's about buffer overflows, and as you'll soon see there really is no need for what the "safe' version is given reentrancy concerns), and not only that, but the replacement, strerror_s(), requires copying and allocation! it's almost like they were TRYING to shove as many error conditions as possible in!
// Oh, and you can't just use _sys_errlist[] to bypass this, because even that has a deprecation warning, telling you to use strerror() instead, which in turn sends you back to strerror_s()!
// Of course, the fact _sys_errlist[] is a thing and that it's deprecated out of security and not reentrancy shows that the error strings returned by strerror()/strerror_s() are static and unchanging throughout the lifetime of the program, so a truly reentrant strerror_s() would just return the raw const string array directly, or a placeholder like "unknown error" otherwise, but that would be too easy!
@ -60,6 +60,8 @@ char *sharedbitsPrefixName(Strncpy)(char *dest, const char *src, size_t n)
// (Furthermore, cppreference.com says there's strerrorlen_s(), but a) fuck C11, and b) MSDN does not concur.)
// So, alas, you'll have to live with just having the error code; sorry.
sharedbitsprivInternalError("error calling strncpy_s(): %d", err);
return 0;
}
return dest;
#else
return strncpy(dest, src, n);

View File

@ -52,7 +52,7 @@ static void checkProgrammerErrorSubtestImpl(struct checkProgrammerErrorParams *p
err = threadNewThread(checkProgrammerErrorThreadProc, p, &thread);
if (err != 0)
// TODO these should only fatal out of the subtest
// TODO these should only fatal out of the subtest (see TODO below)
TestFatalfFull(p->file, p->line, "error creating thread: " threadSysErrorFmt, threadSysErrorFmtArg(err));
err = threadThreadWaitAndFree(thread);
if (err != 0)

View File

@ -63,5 +63,4 @@ void uiprivReportError(const char *prefix, const char *msg, const char *suffix,
{
g_critical("%s: %s. %s", prefix, msg, suffix);
G_BREAKPOINT();
abort(); // we shouldn't reach here
}

View File

@ -42,7 +42,6 @@ static inline void setHInstance(void)
ICC_DATE_CLASSES | /* date/time picker */ \
0)
// TODO add format string warning detection to all these functions, where available
#define uiprivInitReturnHRESULT(err, msg, hr) uiprivInitReturnErrorf(err, "%s: 0x%08I32X", msg, hr)
static DWORD mainThread;
@ -106,8 +105,10 @@ void uiMain(void)
hr = uiprivHrGetMessageW(&msg, NULL, 0, 0);
if (hr == S_FALSE) // WM_QUIT
return;
if (hr != S_OK)
if (hr != S_OK) {
uiprivInternalError("GetMessageW() failed in uiMain(): 0x%08I32X", hr);
return;
}
// TODO IsDialogMessage()
TranslateMessage(&msg);
DispatchMessageW(&msg);
@ -148,5 +149,4 @@ void uiprivReportError(const char *prefix, const char *msg, const char *suffix,
OutputDebugStringA(suffix);
OutputDebugStringA("\n");
DebugBreak();
abort(); // we shouldn't reach here
}