From a16cd4b851d30a8b9389de6085b391e77f0a349e Mon Sep 17 00:00:00 2001 From: David Robillard Date: Thu, 21 Apr 2022 17:53:38 -0400 Subject: Improve error handling --- .clang-format | 1 + src/attributes.h | 7 ++++ src/implementation.c | 56 ++++++++++++++++++------------- src/implementation.h | 14 +++++--- src/types.h | 6 ++++ src/win.h | 6 +++- src/x11.c | 93 +++++++++++++++++++++++++++++++--------------------- src/x11.h | 2 ++ src/x11_gl.c | 34 +++++++++++-------- 9 files changed, 140 insertions(+), 79 deletions(-) diff --git a/.clang-format b/.clang-format index 7b30bd2..4c1027c 100644 --- a/.clang-format +++ b/.clang-format @@ -24,5 +24,6 @@ StatementMacros: - PUGL_CONST_FUNC - PUGL_DEPRECATED_BY - PUGL_UNUSED + - PUGL_WARN_UNUSED_RESULT - _Pragma ... diff --git a/src/attributes.h b/src/attributes.h index 684ee27..578dd2c 100644 --- a/src/attributes.h +++ b/src/attributes.h @@ -13,4 +13,11 @@ # define PUGL_UNUSED(name) name #endif +// Unused result macro to warn when an important return status is ignored +#ifndef _MSC_VER +# define PUGL_WARN_UNUSED_RESULT __attribute__((warn_unused_result)) +#else +# define PUGL_WARN_UNUSED_RESULT +#endif + #endif // PUGL_SRC_ATTRIBUTES_H diff --git a/src/implementation.c b/src/implementation.c index c3cf36b..afc16ab 100644 --- a/src/implementation.c +++ b/src/implementation.c @@ -1,4 +1,4 @@ -// Copyright 2012-2020 David Robillard +// Copyright 2012-2022 David Robillard // SPDX-License-Identifier: ISC #include "implementation.h" @@ -378,7 +378,7 @@ puglMustConfigure(PuglView* view, const PuglConfigureEvent* configure) return memcmp(configure, &view->lastConfigure, sizeof(PuglConfigureEvent)); } -void +PuglStatus puglDispatchSimpleEvent(PuglView* view, const PuglEventType type) { assert(type == PUGL_CREATE || type == PUGL_DESTROY || type == PUGL_MAP || @@ -386,12 +386,14 @@ puglDispatchSimpleEvent(PuglView* view, const PuglEventType type) type == PUGL_LOOP_ENTER || type == PUGL_LOOP_LEAVE); const PuglEvent event = {{type, 0}}; - puglDispatchEvent(view, &event); + return puglDispatchEvent(view, &event); } -void +PuglStatus puglConfigure(PuglView* view, const PuglEvent* event) { + PuglStatus st = PUGL_SUCCESS; + assert(event->type == PUGL_CONFIGURE); view->frame.x = event->configure.x; @@ -400,58 +402,68 @@ puglConfigure(PuglView* view, const PuglEvent* event) view->frame.height = event->configure.height; if (puglMustConfigure(view, &event->configure)) { - view->eventFunc(view, event); + st = view->eventFunc(view, event); view->lastConfigure = event->configure; } + + return st; } -void +PuglStatus puglExpose(PuglView* view, const PuglEvent* event) { - if (event->expose.width > 0.0 && event->expose.height > 0.0) { - view->eventFunc(view, event); - } + return (event->expose.width > 0.0 && event->expose.height > 0.0) + ? view->eventFunc(view, event) + : PUGL_SUCCESS; } -void +PuglStatus puglDispatchEvent(PuglView* view, const PuglEvent* event) { + PuglStatus st0 = PUGL_SUCCESS; + PuglStatus st1 = PUGL_SUCCESS; + switch (event->type) { case PUGL_NOTHING: break; case PUGL_CREATE: case PUGL_DESTROY: - view->backend->enter(view, NULL); - view->eventFunc(view, event); - view->backend->leave(view, NULL); + if (!(st0 = view->backend->enter(view, NULL))) { + st0 = view->eventFunc(view, event); + st1 = view->backend->leave(view, NULL); + } break; case PUGL_CONFIGURE: if (puglMustConfigure(view, &event->configure)) { - view->backend->enter(view, NULL); - puglConfigure(view, event); - view->backend->leave(view, NULL); + if (!(st0 = view->backend->enter(view, NULL))) { + st0 = puglConfigure(view, event); + st1 = view->backend->leave(view, NULL); + } } break; case PUGL_MAP: if (!view->visible) { view->visible = true; - view->eventFunc(view, event); + st0 = view->eventFunc(view, event); } break; case PUGL_UNMAP: if (view->visible) { view->visible = false; - view->eventFunc(view, event); + st0 = view->eventFunc(view, event); } break; case PUGL_EXPOSE: - view->backend->enter(view, &event->expose); - puglExpose(view, event); - view->backend->leave(view, &event->expose); + if (!(st0 = view->backend->enter(view, &event->expose))) { + st0 = puglExpose(view, event); + st1 = view->backend->leave(view, &event->expose); + } break; default: - view->eventFunc(view, event); + st0 = view->eventFunc(view, event); } + + return st0 ? st0 : st1; } const void* diff --git a/src/implementation.h b/src/implementation.h index 77ff606..0fcba02 100644 --- a/src/implementation.h +++ b/src/implementation.h @@ -1,9 +1,10 @@ -// Copyright 2012-2020 David Robillard +// Copyright 2012-2022 David Robillard // SPDX-License-Identifier: ISC #ifndef PUGL_IMPLEMENTATION_H #define PUGL_IMPLEMENTATION_H +#include "attributes.h" #include "types.h" #include "pugl/pugl.h" @@ -42,19 +43,21 @@ uint32_t puglDecodeUTF8(const uint8_t* buf); /// Dispatch an event with a simple `type` to `view` -void +PuglStatus puglDispatchSimpleEvent(PuglView* view, PuglEventType type); /// Process configure event while already in the graphics context -void +PUGL_WARN_UNUSED_RESULT +PuglStatus puglConfigure(PuglView* view, const PuglEvent* event); /// Process expose event while already in the graphics context -void +PUGL_WARN_UNUSED_RESULT +PuglStatus puglExpose(PuglView* view, const PuglEvent* event); /// Dispatch `event` to `view`, entering graphics context if necessary -void +PuglStatus puglDispatchEvent(PuglView* view, const PuglEvent* event); /// Set internal (stored in view) clipboard contents @@ -62,6 +65,7 @@ const void* puglGetInternalClipboard(const PuglView* view, const char** type, size_t* len); /// Set internal (stored in view) clipboard contents +PUGL_WARN_UNUSED_RESULT PuglStatus puglSetInternalClipboard(PuglView* view, const char* type, diff --git a/src/types.h b/src/types.h index 107e359..67e9eef 100644 --- a/src/types.h +++ b/src/types.h @@ -4,6 +4,8 @@ #ifndef PUGL_SRC_TYPES_H #define PUGL_SRC_TYPES_H +#include "attributes.h" + #include "pugl/pugl.h" #include @@ -68,18 +70,22 @@ typedef void PuglSurface; /// Graphics backend interface struct PuglBackendImpl { /// Get visual information from display and setup view as necessary + PUGL_WARN_UNUSED_RESULT PuglStatus (*configure)(PuglView*); /// Create surface and drawing context + PUGL_WARN_UNUSED_RESULT PuglStatus (*create)(PuglView*); /// Destroy surface and drawing context void (*destroy)(PuglView*); /// Enter drawing context, for drawing if expose is non-null + PUGL_WARN_UNUSED_RESULT PuglStatus (*enter)(PuglView*, const PuglExposeEvent*); /// Leave drawing context, after drawing if expose is non-null + PUGL_WARN_UNUSED_RESULT PuglStatus (*leave)(PuglView*, const PuglExposeEvent*); /// Return the puglGetContext() handle for the application, if any diff --git a/src/win.h b/src/win.h index 130eabf..3f879c2 100644 --- a/src/win.h +++ b/src/win.h @@ -1,4 +1,4 @@ -// Copyright 2012-2021 David Robillard +// Copyright 2012-2022 David Robillard // SPDX-License-Identifier: ISC #ifndef PUGL_SRC_WIN_H @@ -33,6 +33,7 @@ PUGL_API PuglWinPFD puglWinGetPixelFormatDescriptor(const PuglHints hints); +PUGL_WARN_UNUSED_RESULT PUGL_API PuglStatus puglWinCreateWindow(PuglView* const view, @@ -40,14 +41,17 @@ puglWinCreateWindow(PuglView* const view, HWND* const hwnd, HDC* const hdc); +PUGL_WARN_UNUSED_RESULT PUGL_API PuglStatus puglWinConfigure(PuglView* view); +PUGL_WARN_UNUSED_RESULT PUGL_API PuglStatus puglWinEnter(PuglView* view, const PuglExposeEvent* expose); +PUGL_WARN_UNUSED_RESULT PUGL_API PuglStatus puglWinLeave(PuglView* view, const PuglExposeEvent* expose); diff --git a/src/x11.c b/src/x11.c index 81a91b1..8a072be 100644 --- a/src/x11.c +++ b/src/x11.c @@ -1,4 +1,4 @@ -// Copyright 2012-2021 David Robillard +// Copyright 2012-2022 David Robillard // Copyright 2013 Robin Gareus // Copyright 2011-2012 Ben Loftis, Harrison Consoles // SPDX-License-Identifier: ISC @@ -9,6 +9,7 @@ #include "x11.h" +#include "attributes.h" #include "implementation.h" #include "types.h" @@ -831,13 +832,14 @@ puglRequestAttention(PuglView* const view) event.xclient.data.l[4] = 0; const Window root = RootWindow(impl->display, impl->screen); - XSendEvent(impl->display, - root, - False, - SubstructureNotifyMask | SubstructureRedirectMask, - &event); - return PUGL_SUCCESS; + return XSendEvent(impl->display, + root, + False, + SubstructureNotifyMask | SubstructureRedirectMask, + &event) + ? PUGL_SUCCESS + : PUGL_UNKNOWN_ERROR; } PuglStatus @@ -964,11 +966,9 @@ puglSendEvent(PuglView* const view, const PuglEvent* const event) XEvent xev = eventToX(view, event); if (xev.type) { - if (XSendEvent(view->impl->display, view->impl->win, False, 0, &xev)) { - return PUGL_SUCCESS; - } - - return PUGL_UNKNOWN_ERROR; + return XSendEvent(view->impl->display, view->impl->win, False, 0, &xev) + ? PUGL_SUCCESS + : PUGL_UNKNOWN_ERROR; } return PUGL_UNSUPPORTED_TYPE; @@ -1029,7 +1029,7 @@ handleSelectionNotify(const PuglWorld* const world, PuglView* const view) XFree(str); } -static void +static PuglStatus handleSelectionRequest(const PuglWorld* const world, PuglView* const view, const XSelectionRequestEvent* const request) @@ -1062,13 +1062,21 @@ handleSelectionRequest(const PuglWorld* const world, note.property = None; } - XSendEvent(world->impl->display, note.requestor, True, 0, (XEvent*)¬e); + return XSendEvent( + world->impl->display, note.requestor, True, 0, (XEvent*)¬e) + ? PUGL_SUCCESS + : PUGL_UNKNOWN_ERROR; } /// Flush pending configure and expose events for all views -static void +PUGL_WARN_UNUSED_RESULT +static PuglStatus flushExposures(PuglWorld* const world) { + PuglStatus st0 = PUGL_SUCCESS; + PuglStatus st1 = PUGL_SUCCESS; + PuglStatus st2 = PUGL_SUCCESS; + for (size_t i = 0; i < world->numViews; ++i) { PuglView* const view = world->views[i]; @@ -1085,20 +1093,23 @@ flushExposures(PuglWorld* const world) view->impl->pendingExpose.type = PUGL_NOTHING; if (expose.type) { - view->backend->enter(view, &expose.expose); + if (!(st0 = view->backend->enter(view, &expose.expose))) { + if (configure.type) { + st0 = puglConfigure(view, &configure); + } - if (configure.type) { - puglConfigure(view, &configure); + st1 = puglExpose(view, &expose); + st2 = view->backend->leave(view, &expose.expose); } - - puglExpose(view, &expose); - view->backend->leave(view, &expose.expose); } else if (configure.type) { - view->backend->enter(view, NULL); - puglConfigure(view, &configure); - view->backend->leave(view, NULL); + if (!(st0 = view->backend->enter(view, NULL))) { + st0 = puglConfigure(view, &configure); + st1 = view->backend->leave(view, NULL); + } } } + + return st0 ? st0 : st1 ? st1 : st2; } static bool @@ -1130,6 +1141,9 @@ handleTimerEvent(PuglWorld* const world, const XEvent xevent) static PuglStatus dispatchX11Events(PuglWorld* const world) { + PuglStatus st0 = PUGL_SUCCESS; + PuglStatus st1 = PUGL_SUCCESS; + const PuglX11Atoms* const atoms = &world->impl->atoms; // Flush output to the server once at the start @@ -1196,15 +1210,15 @@ dispatchX11Events(PuglWorld* const world) configureEvent.configure.height = (double)attrs.height; // Dispatch an initial configure (if necessary), then the map event - puglDispatchEvent(view, &configureEvent); - puglDispatchEvent(view, &event); + st0 = puglDispatchEvent(view, &configureEvent); + st1 = puglDispatchEvent(view, &event); } else { // Dispatch event to application immediately - puglDispatchEvent(view, &event); + st0 = puglDispatchEvent(view, &event); } } - return PUGL_SUCCESS; + return st0 ? st0 : st1; } #ifndef PUGL_DISABLE_DEPRECATED @@ -1219,32 +1233,33 @@ PuglStatus puglUpdate(PuglWorld* const world, const double timeout) { const double startTime = puglGetTime(world); - PuglStatus st = PUGL_SUCCESS; + PuglStatus st0 = PUGL_SUCCESS; + PuglStatus st1 = PUGL_SUCCESS; world->impl->dispatchingEvents = true; if (timeout < 0.0) { - st = pollX11Socket(world, timeout); - st = st ? st : dispatchX11Events(world); + st0 = pollX11Socket(world, timeout); + st0 = st0 ? st0 : dispatchX11Events(world); } else if (timeout <= 0.001) { - st = dispatchX11Events(world); + st0 = dispatchX11Events(world); } else { const double endTime = startTime + timeout - 0.001; double t = startTime; - while (!st && t < endTime) { - if (!(st = pollX11Socket(world, endTime - t))) { - st = dispatchX11Events(world); + while (!st0 && t < endTime) { + if (!(st0 = pollX11Socket(world, endTime - t))) { + st0 = dispatchX11Events(world); } t = puglGetTime(world); } } - flushExposures(world); + st1 = flushExposures(world); world->impl->dispatchingEvents = false; - return st; + return st0 ? st0 : st1; } double @@ -1478,7 +1493,9 @@ puglX11Configure(PuglView* view) int n = 0; pat.screen = impl->screen; - impl->vi = XGetVisualInfo(impl->display, VisualScreenMask, &pat, &n); + if (!(impl->vi = XGetVisualInfo(impl->display, VisualScreenMask, &pat, &n))) { + return PUGL_BAD_CONFIGURATION; + } view->hints[PUGL_RED_BITS] = impl->vi->bits_per_rgb; view->hints[PUGL_GREEN_BITS] = impl->vi->bits_per_rgb; diff --git a/src/x11.h b/src/x11.h index e55355a..f85b227 100644 --- a/src/x11.h +++ b/src/x11.h @@ -4,6 +4,7 @@ #ifndef PUGL_SRC_X11_H #define PUGL_SRC_X11_H +#include "attributes.h" #include "types.h" #include "pugl/pugl.h" @@ -60,6 +61,7 @@ struct PuglInternalsImpl { #endif }; +PUGL_WARN_UNUSED_RESULT PUGL_API PuglStatus puglX11Configure(PuglView* view); diff --git a/src/x11_gl.c b/src/x11_gl.c index 934fc18..1ebe829 100644 --- a/src/x11_gl.c +++ b/src/x11_gl.c @@ -98,14 +98,18 @@ puglX11GlConfigure(PuglView* view) return PUGL_SUCCESS; } +PUGL_WARN_UNUSED_RESULT static PuglStatus puglX11GlEnter(PuglView* view, const PuglExposeEvent* PUGL_UNUSED(expose)) { PuglX11GlSurface* surface = (PuglX11GlSurface*)view->impl->surface; - glXMakeCurrent(view->impl->display, view->impl->win, surface->ctx); - return PUGL_SUCCESS; + + return glXMakeCurrent(view->impl->display, view->impl->win, surface->ctx) + ? PUGL_SUCCESS + : PUGL_FAILURE; } +PUGL_WARN_UNUSED_RESULT static PuglStatus puglX11GlLeave(PuglView* view, const PuglExposeEvent* expose) { @@ -113,9 +117,8 @@ puglX11GlLeave(PuglView* view, const PuglExposeEvent* expose) glXSwapBuffers(view->impl->display, view->impl->win); } - glXMakeCurrent(view->impl->display, None, NULL); - - return PUGL_SUCCESS; + return glXMakeCurrent(view->impl->display, None, NULL) ? PUGL_SUCCESS + : PUGL_FAILURE; } static PuglStatus @@ -125,6 +128,7 @@ puglX11GlCreate(PuglView* view) PuglX11GlSurface* const surface = (PuglX11GlSurface*)impl->surface; Display* const display = impl->display; GLXFBConfig fb_config = surface->fb_config; + PuglStatus st = PUGL_SUCCESS; const int ctx_attrs[] = { GLX_CONTEXT_MAJOR_VERSION_ARB, @@ -171,7 +175,9 @@ puglX11GlCreate(PuglView* view) (const uint8_t*)"glXSwapIntervalEXT"); // Note that some drivers (NVidia) require the context to be entered here - puglX11GlEnter(view, NULL); + if ((st = puglX11GlEnter(view, NULL))) { + return st; + } // Set the swap interval if the user requested a specific value if (view->hints[PUGL_SWAP_INTERVAL] != PUGL_DONT_CARE) { @@ -184,15 +190,17 @@ puglX11GlCreate(PuglView* view) GLX_SWAP_INTERVAL_EXT, (unsigned int*)&view->hints[PUGL_SWAP_INTERVAL]); - puglX11GlLeave(view, NULL); + if ((st = puglX11GlLeave(view, NULL))) { + return st; + } } - glXGetConfig(impl->display, - impl->vi, - GLX_DOUBLEBUFFER, - &view->hints[PUGL_DOUBLE_BUFFER]); - - return PUGL_SUCCESS; + return !glXGetConfig(impl->display, + impl->vi, + GLX_DOUBLEBUFFER, + &view->hints[PUGL_DOUBLE_BUFFER]) + ? PUGL_SUCCESS + : PUGL_UNKNOWN_ERROR; } static void -- cgit v1.2.1