diff --git a/common/main.c b/common/main.c index a0624761..ac3a56f2 100644 --- a/common/main.c +++ b/common/main.c @@ -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] = '.'; diff --git a/darwin/main.m b/darwin/main.m index 8bc8a8bd..e26b1566 100644 --- a/darwin/main.m +++ b/darwin/main.m @@ -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__ diff --git a/haiku/main.cpp b/haiku/main.cpp index ed5327bb..b610abdb 100644 --- a/haiku/main.cpp +++ b/haiku/main.cpp @@ -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 } diff --git a/sharedbits/alloc_impl.h b/sharedbits/alloc_impl.h index 6e6e3e05..e9bb2cac 100644 --- a/sharedbits/alloc_impl.h +++ b/sharedbits/alloc_impl.h @@ -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; diff --git a/sharedbits/strsafe_impl.h b/sharedbits/strsafe_impl.h index d2b11693..b5276591 100644 --- a/sharedbits/strsafe_impl.h +++ b/sharedbits/strsafe_impl.h @@ -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); diff --git a/test/errors.c b/test/errors.c index 36f30226..df0061b5 100644 --- a/test/errors.c +++ b/test/errors.c @@ -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) diff --git a/unix/main.c b/unix/main.c index 02099917..7c82465e 100644 --- a/unix/main.c +++ b/unix/main.c @@ -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 } diff --git a/windows/main.cpp b/windows/main.cpp index 46ccdbee..b930db73 100644 --- a/windows/main.cpp +++ b/windows/main.cpp @@ -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 }