From 36c6eeaf811ff5c4cc8486401d01f40c67d947c6 Mon Sep 17 00:00:00 2001 From: Scott Duensing Date: Wed, 22 Apr 2026 21:26:49 -0500 Subject: [PATCH] Fixes for issues found by GCC analyzer. --- analyze.sh | 54 +++++++++++++++++++ src/libs/kpunch/libdvx/dvxWgtP.h | 6 +++ src/libs/kpunch/libdvx/widgetCore.c | 34 ++++++++++-- src/widgets/kpunch/canvas/widgetCanvas.c | 40 ++++++++------ src/widgets/kpunch/image/widgetImage.c | 23 +++++--- .../kpunch/imageButton/widgetImageButton.c | 24 ++++++--- .../kpunch/progressBar/widgetProgressBar.c | 38 ++++++++----- .../kpunch/separator/widgetSeparator.c | 46 ++++++++++------ src/widgets/kpunch/slider/widgetSlider.c | 21 +++++--- src/widgets/kpunch/splitter/widgetSplitter.c | 19 ++++--- .../kpunch/tabControl/widgetTabControl.c | 36 +++++++++---- src/widgets/kpunch/timer/widgetTimer.c | 29 ++++++---- 12 files changed, 270 insertions(+), 100 deletions(-) create mode 100755 analyze.sh diff --git a/analyze.sh b/analyze.sh new file mode 100755 index 0000000..e32bc0c --- /dev/null +++ b/analyze.sh @@ -0,0 +1,54 @@ +#!/bin/bash +# +# analyze.sh -- run the DJGPP build under gcc's -fanalyzer. +# +# Usage: +# ./analyze.sh # analyse the whole build (slow: 2-5x) +# ./analyze.sh 2>&1 | tee analyze.log +# +# How it works: +# +# The project Makefiles hardcode -Werror in CFLAGS. gcc evaluates +# diagnostic flags in order, so a trailing -Wno-error (appended +# AFTER CFLAGS at compile time) demotes analyzer findings back to +# warnings -- otherwise the first analyzer hit aborts the build. +# +# We do that by setting CC to a wrapper that execs the real compiler +# with -fanalyzer prepended and -Wno-error appended. make -k keeps +# building after individual failures so we see every hit in one run. +# +# Runs the existing top-level Makefile with an override CC; no +# Makefile edits required. Output goes to stderr; redirect with +# '2>&1 | tee analyze.log' to capture. + +set -u + +cd "$(dirname "$0")" + +REAL_CC="$HOME/djgpp/djgpp/bin/i586-pc-msdosdjgpp-gcc" + +if [ ! -x "$REAL_CC" ]; then + echo "analyze.sh: $REAL_CC not executable" >&2 + exit 1 +fi + +# Build the wrapper in a temp dir that the spawned makes can see. +WRAP_DIR="$(mktemp -d)" +trap 'rm -rf "$WRAP_DIR"' EXIT +WRAP="$WRAP_DIR/analyze-cc" + +cat > "$WRAP" <&2 +echo "analyze.sh: this will be slow (2-5x normal build time)." >&2 + +# -k = keep going after errors so we collect every analyzer hit +# CC=... overrides the assignment in each submakefile +make -k CC="$WRAP" 2>&1 + +echo "analyze.sh: done. Grep for 'Wanalyzer' to see findings." >&2 diff --git a/src/libs/kpunch/libdvx/dvxWgtP.h b/src/libs/kpunch/libdvx/dvxWgtP.h index e5df10b..cd53bdc 100644 --- a/src/libs/kpunch/libdvx/dvxWgtP.h +++ b/src/libs/kpunch/libdvx/dvxWgtP.h @@ -142,6 +142,12 @@ void widgetDestroyChildren(WidgetT *w); // Allocation WidgetT *widgetAlloc(WidgetT *parent, int32_t type); +// Undo a successful widgetAlloc when a subsequent data-struct +// allocation fails. Detaches from parent, removes from the poll +// list, frees the widget struct. Call only before w->data has +// been set; once data is attached, use wgtDestroy instead. +void widgetAllocRollback(WidgetT *w); + // Allocate a widget of the given type PLUS a data struct of dataSize // bytes. The data struct must begin with a `const char *text` field // (WCLASS_HAS_TEXT semantics); this field is set to strdup(text) and diff --git a/src/libs/kpunch/libdvx/widgetCore.c b/src/libs/kpunch/libdvx/widgetCore.c index 119b2a1..d347d15 100644 --- a/src/libs/kpunch/libdvx/widgetCore.c +++ b/src/libs/kpunch/libdvx/widgetCore.c @@ -257,6 +257,34 @@ WidgetT *widgetAlloc(WidgetT *parent, int32_t type) { } +// Undo a successful widgetAlloc for a widget whose data-struct +// allocation subsequently failed. Detaches from parent, removes +// from the poll list, and frees the widget struct. Safe only +// before w->data has been set -- once data is attached, use +// wgtDestroy to run the widget's full teardown path. +void widgetAllocRollback(WidgetT *w) { + if (!w) { + return; + } + + if (w->parent) { + widgetRemoveChild(w->parent, w); + } + + if (w->wclass && (w->wclass->flags & WCLASS_NEEDS_POLL)) { + for (int32_t i = 0; i < sPollWidgetCount; i++) { + if (sPollWidgets[i] == w) { + arrdel(sPollWidgets, i); + sPollWidgetCount = (int32_t)arrlen(sPollWidgets); + break; + } + } + } + + free(w); +} + + WidgetT *widgetAllocWithText(WidgetT *parent, int32_t type, size_t dataSize, const char *text) { WidgetT *w = widgetAlloc(parent, type); @@ -267,11 +295,9 @@ WidgetT *widgetAllocWithText(WidgetT *parent, int32_t type, size_t dataSize, con void *data = calloc(1, dataSize); if (!data) { - // Widget itself is already in the tree; leave it rather than - // attempting a partial rollback (widget destroy is idempotent - // via the parent teardown). dvxLog("Widget: failed to allocate %u-byte data for type %d", (unsigned)dataSize, type); - return w; + widgetAllocRollback(w); + return NULL; } // The data struct must begin with a `const char *text` field. diff --git a/src/widgets/kpunch/canvas/widgetCanvas.c b/src/widgets/kpunch/canvas/widgetCanvas.c index 94d8263..91c74e9 100644 --- a/src/widgets/kpunch/canvas/widgetCanvas.c +++ b/src/widgets/kpunch/canvas/widgetCanvas.c @@ -355,26 +355,32 @@ WidgetT *wgtCanvas(WidgetT *parent, int32_t w, int32_t h) { WidgetT *wgt = widgetAlloc(parent, sTypeId); - if (wgt) { - wgt->contentOffX = CANVAS_BORDER; - wgt->contentOffY = CANVAS_BORDER; - - CanvasDataT *cd = (CanvasDataT *)calloc(1, sizeof(CanvasDataT)); - - cd->pixelData = data; - cd->canvasW = w; - cd->canvasH = h; - cd->canvasPitch = pitch; - cd->canvasBpp = bpp; - cd->penColor = packColor(d, 0, 0, 0); - cd->penSize = 1; - cd->lastX = -1; - cd->lastY = -1; - wgt->data = cd; - } else { + if (!wgt) { free(data); + return NULL; } + wgt->contentOffX = CANVAS_BORDER; + wgt->contentOffY = CANVAS_BORDER; + + CanvasDataT *cd = (CanvasDataT *)calloc(1, sizeof(CanvasDataT)); + + if (!cd) { + free(data); + widgetAllocRollback(wgt); + return NULL; + } + + cd->pixelData = data; + cd->canvasW = w; + cd->canvasH = h; + cd->canvasPitch = pitch; + cd->canvasBpp = bpp; + cd->penColor = packColor(d, 0, 0, 0); + cd->penSize = 1; + cd->lastX = -1; + cd->lastY = -1; + wgt->data = cd; return wgt; } diff --git a/src/widgets/kpunch/image/widgetImage.c b/src/widgets/kpunch/image/widgetImage.c index 6fedd99..a03467f 100644 --- a/src/widgets/kpunch/image/widgetImage.c +++ b/src/widgets/kpunch/image/widgetImage.c @@ -81,16 +81,23 @@ void widgetImagePaint(WidgetT *w, DisplayT *disp, const BlitOpsT *ops, const Bit WidgetT *wgtImage(WidgetT *parent, uint8_t *pixelData, int32_t w, int32_t h, int32_t pitch) { WidgetT *wgt = widgetAlloc(parent, sTypeId); - if (wgt) { - ImageDataT *d = calloc(1, sizeof(ImageDataT)); - d->pixelData = pixelData; - d->imgW = w; - d->imgH = h; - d->imgPitch = pitch; - d->pressed = false; - wgt->data = d; + if (!wgt) { + return NULL; } + ImageDataT *d = calloc(1, sizeof(ImageDataT)); + + if (!d) { + widgetAllocRollback(wgt); + return NULL; + } + + d->pixelData = pixelData; + d->imgW = w; + d->imgH = h; + d->imgPitch = pitch; + d->pressed = false; + wgt->data = d; return wgt; } diff --git a/src/widgets/kpunch/imageButton/widgetImageButton.c b/src/widgets/kpunch/imageButton/widgetImageButton.c index a7f3dca..e7aea54 100644 --- a/src/widgets/kpunch/imageButton/widgetImageButton.c +++ b/src/widgets/kpunch/imageButton/widgetImageButton.c @@ -81,17 +81,25 @@ WidgetT *wgtImageButton(WidgetT *parent, uint8_t *pixelData, int32_t w, int32_t WidgetT *wgt = widgetAlloc(parent, sTypeId); - if (wgt) { - ImageButtonDataT *d = calloc(1, sizeof(ImageButtonDataT)); - d->pixelData = pixelData; - d->imgW = w; - d->imgH = h; - d->imgPitch = pitch; - wgt->data = d; - } else { + if (!wgt) { free(pixelData); + return NULL; } + ImageButtonDataT *d = calloc(1, sizeof(ImageButtonDataT)); + + if (!d) { + free(pixelData); + widgetAllocRollback(wgt); + return NULL; + } + + d->pixelData = pixelData; + d->imgW = w; + d->imgH = h; + d->imgPitch = pitch; + wgt->data = d; + return wgt; } diff --git a/src/widgets/kpunch/progressBar/widgetProgressBar.c b/src/widgets/kpunch/progressBar/widgetProgressBar.c index 7176933..9d1104f 100644 --- a/src/widgets/kpunch/progressBar/widgetProgressBar.c +++ b/src/widgets/kpunch/progressBar/widgetProgressBar.c @@ -68,14 +68,21 @@ void widgetProgressBarPaint(WidgetT *w, DisplayT *disp, const BlitOpsT *ops, WidgetT *wgtProgressBar(WidgetT *parent) { WidgetT *w = widgetAlloc(parent, sTypeId); - if (w) { - ProgressBarDataT *d = calloc(1, sizeof(ProgressBarDataT)); - d->value = 0; - d->maxValue = 100; - d->vertical = false; - w->data = d; + if (!w) { + return NULL; } + ProgressBarDataT *d = calloc(1, sizeof(ProgressBarDataT)); + + if (!d) { + widgetAllocRollback(w); + return NULL; + } + + d->value = 0; + d->maxValue = 100; + d->vertical = false; + w->data = d; return w; } @@ -108,14 +115,21 @@ void wgtProgressBarSetValue(WidgetT *w, int32_t value) { WidgetT *wgtProgressBarV(WidgetT *parent) { WidgetT *w = widgetAlloc(parent, sTypeId); - if (w) { - ProgressBarDataT *d = calloc(1, sizeof(ProgressBarDataT)); - d->value = 0; - d->maxValue = 100; - d->vertical = true; - w->data = d; + if (!w) { + return NULL; } + ProgressBarDataT *d = calloc(1, sizeof(ProgressBarDataT)); + + if (!d) { + widgetAllocRollback(w); + return NULL; + } + + d->value = 0; + d->maxValue = 100; + d->vertical = true; + w->data = d; return w; } diff --git a/src/widgets/kpunch/separator/widgetSeparator.c b/src/widgets/kpunch/separator/widgetSeparator.c index 36207f9..df6185d 100644 --- a/src/widgets/kpunch/separator/widgetSeparator.c +++ b/src/widgets/kpunch/separator/widgetSeparator.c @@ -65,20 +65,27 @@ void widgetSeparatorPaint(WidgetT *w, DisplayT *d, const BlitOpsT *ops, WidgetT *wgtHSeparator(WidgetT *parent) { WidgetT *w = widgetAlloc(parent, sTypeId); - if (w) { - SeparatorDataT *d = calloc(1, sizeof(SeparatorDataT)); - // Auto-orient to the parent: horizontal container -> vertical - // divider, vertical container -> horizontal divider. This - // lets a single "Line" basName work correctly in both - // toolbars and menus without requiring callers to pick - // between HSeparator and VSeparator by widget type. - if (parent && widgetIsHorizContainer(parent->type)) { - d->vertical = true; - } - - w->data = d; + if (!w) { + return NULL; } + SeparatorDataT *d = calloc(1, sizeof(SeparatorDataT)); + + if (!d) { + widgetAllocRollback(w); + return NULL; + } + + // Auto-orient to the parent: horizontal container -> vertical + // divider, vertical container -> horizontal divider. This + // lets a single "Line" basName work correctly in both + // toolbars and menus without requiring callers to pick + // between HSeparator and VSeparator by widget type. + if (parent && widgetIsHorizContainer(parent->type)) { + d->vertical = true; + } + + w->data = d; return w; } @@ -126,12 +133,19 @@ void wgtRegister(void) { WidgetT *wgtVSeparator(WidgetT *parent) { WidgetT *w = widgetAlloc(parent, sTypeId); - if (w) { - SeparatorDataT *d = calloc(1, sizeof(SeparatorDataT)); - w->data = d; - d->vertical = true; + if (!w) { + return NULL; } + SeparatorDataT *d = calloc(1, sizeof(SeparatorDataT)); + + if (!d) { + widgetAllocRollback(w); + return NULL; + } + + w->data = d; + d->vertical = true; return w; } diff --git a/src/widgets/kpunch/slider/widgetSlider.c b/src/widgets/kpunch/slider/widgetSlider.c index a6b6997..8440f0d 100644 --- a/src/widgets/kpunch/slider/widgetSlider.c +++ b/src/widgets/kpunch/slider/widgetSlider.c @@ -132,15 +132,22 @@ void wgtRegister(void) { WidgetT *wgtSlider(WidgetT *parent, int32_t minVal, int32_t maxVal) { WidgetT *w = widgetAlloc(parent, sTypeId); - if (w) { - SliderDataT *d = (SliderDataT *)calloc(1, sizeof(SliderDataT)); - d->value = minVal; - d->minValue = minVal; - d->maxValue = maxVal; - w->data = d; - w->weight = WGT_WEIGHT_FILL; + if (!w) { + return NULL; } + SliderDataT *d = (SliderDataT *)calloc(1, sizeof(SliderDataT)); + + if (!d) { + widgetAllocRollback(w); + return NULL; + } + + d->value = minVal; + d->minValue = minVal; + d->maxValue = maxVal; + w->data = d; + w->weight = WGT_WEIGHT_FILL; return w; } diff --git a/src/widgets/kpunch/splitter/widgetSplitter.c b/src/widgets/kpunch/splitter/widgetSplitter.c index c4d6c58..dda3c50 100644 --- a/src/widgets/kpunch/splitter/widgetSplitter.c +++ b/src/widgets/kpunch/splitter/widgetSplitter.c @@ -170,14 +170,21 @@ void wgtRegister(void) { WidgetT *wgtSplitter(WidgetT *parent, bool vertical) { WidgetT *w = widgetAlloc(parent, sTypeId); - if (w) { - SplitterDataT *d = (SplitterDataT *)calloc(1, sizeof(SplitterDataT)); - d->vertical = vertical; - d->dividerPos = 0; - w->data = d; - w->weight = WGT_WEIGHT_FILL; + if (!w) { + return NULL; } + SplitterDataT *d = (SplitterDataT *)calloc(1, sizeof(SplitterDataT)); + + if (!d) { + widgetAllocRollback(w); + return NULL; + } + + d->vertical = vertical; + d->dividerPos = 0; + w->data = d; + w->weight = WGT_WEIGHT_FILL; return w; } diff --git a/src/widgets/kpunch/tabControl/widgetTabControl.c b/src/widgets/kpunch/tabControl/widgetTabControl.c index 29139cb..4c78250 100644 --- a/src/widgets/kpunch/tabControl/widgetTabControl.c +++ b/src/widgets/kpunch/tabControl/widgetTabControl.c @@ -193,14 +193,21 @@ static bool tabNeedScroll(const WidgetT *w, const BitmapFontT *font) { WidgetT *wgtTabControl(WidgetT *parent) { WidgetT *w = widgetAlloc(parent, sTabControlTypeId); - if (w) { - TabControlDataT *d = calloc(1, sizeof(TabControlDataT)); - d->activeTab = 0; - d->scrollOffset = 0; - w->data = d; - w->weight = WGT_WEIGHT_FILL; + if (!w) { + return NULL; } + TabControlDataT *d = calloc(1, sizeof(TabControlDataT)); + + if (!d) { + widgetAllocRollback(w); + return NULL; + } + + d->activeTab = 0; + d->scrollOffset = 0; + w->data = d; + w->weight = WGT_WEIGHT_FILL; return w; } @@ -225,13 +232,20 @@ void wgtTabControlSetActive(WidgetT *w, int32_t idx) { WidgetT *wgtTabPage(WidgetT *parent, const char *title) { WidgetT *w = widgetAlloc(parent, sTabPageTypeId); - if (w) { - TabPageDataT *d = calloc(1, sizeof(TabPageDataT)); - d->title = strdup(title ? title : ""); - w->data = d; - w->accelKey = accelParse(d->title); + if (!w) { + return NULL; } + TabPageDataT *d = calloc(1, sizeof(TabPageDataT)); + + if (!d) { + widgetAllocRollback(w); + return NULL; + } + + d->title = strdup(title ? title : ""); + w->data = d; + w->accelKey = accelParse(d->title); return w; } diff --git a/src/widgets/kpunch/timer/widgetTimer.c b/src/widgets/kpunch/timer/widgetTimer.c index 6c6316d..d82e953 100644 --- a/src/widgets/kpunch/timer/widgetTimer.c +++ b/src/widgets/kpunch/timer/widgetTimer.c @@ -101,19 +101,26 @@ static void widgetTimerDestroy(WidgetT *w) { WidgetT *wgtTimer(WidgetT *parent, int32_t intervalMs, bool repeat) { WidgetT *w = widgetAlloc(parent, sTypeId); - if (w) { - TimerDataT *d = calloc(1, sizeof(TimerDataT)); - w->data = d; - w->visible = false; - d->intervalMs = intervalMs; - d->repeat = repeat; - // Match VB Timer default (Enabled=True on create). Callers can - // Stop() if they want it dormant. - d->running = true; - d->lastFire = clock(); - timerAddToActiveList(w); + if (!w) { + return NULL; } + TimerDataT *d = calloc(1, sizeof(TimerDataT)); + + if (!d) { + widgetAllocRollback(w); + return NULL; + } + + w->data = d; + w->visible = false; + d->intervalMs = intervalMs; + d->repeat = repeat; + // Match VB Timer default (Enabled=True on create). Callers can + // Stop() if they want it dormant. + d->running = true; + d->lastFire = clock(); + timerAddToActiveList(w); return w; }