From 74c87caac37e7cd1855a18fa646f3071915cff05 Mon Sep 17 00:00:00 2001 From: Ingo Paschke Date: Tue, 12 May 2026 11:35:27 +0200 Subject: [PATCH] Amiga: fix bitmap/IFF resource handling in winchar.c - MyAllocBitMap left bm->bm.Planes[] uninitialized; InitBitMap only fills BytesPerRow/Rows/Flags/Depth, not Planes[]. If AllocRaster fails mid-loop, MyFreeBitMap was iterating up to Depth and would pass uninitialized stack-garbage pointers to FreeRaster. Zero Planes[] before the alloc loop. - ReadImageFile leaked iffparse.library, the IFFHandle, the DOS file handle, and any open-IFF state on every panic path. On AmigaOS those handles are not auto-reclaimed when the process dies, so each failure stranded resources until reboot. Restructure to a single cleanup label and free in reverse-acquisition order before panicking. - OpenIFF returns an error code that was being thrown away, so a failed open would feed corrupt state to ParseIFF. Check and bail. --- sys/amiga/winchar.c | 68 +++++++++++++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 17 deletions(-) diff --git a/sys/amiga/winchar.c b/sys/amiga/winchar.c index b84561311..9636ba3e7 100644 --- a/sys/amiga/winchar.c +++ b/sys/amiga/winchar.c @@ -79,36 +79,54 @@ struct BitMap *tileimg, *tile; BitMapHeader ReadImageFile(const char *filename, struct BitMap **bmp) { - BitMapHeader *bmhd, bmhds; + BitMapHeader *bmhd, bmhds = { 0 }; int j, np; - struct IFFHandle *iff; + long err; + struct IFFHandle *iff = NULL; struct StoredProperty *prop; + int iff_opened = 0; + const char *errfmt = NULL; + long errcode = 0; IFFParseBase = OpenLibrary("iffparse.library", 0L); if (!IFFParseBase) panic("No iffparse.library"); iff = AllocIFF(); - if (!iff) - panic("can't start IFF processing"); + if (!iff) { + errfmt = "can't start IFF processing"; + goto cleanup; + } iff->iff_Stream = Open(filename, MODE_OLDFILE); - if (iff->iff_Stream == 0) - panic("Can't open %s", filename); + if (iff->iff_Stream == 0) { + errfmt = "Can't open %s"; + goto cleanup; + } InitIFFasDOS(iff); - OpenIFF(iff, IFFF_READ); + if ((err = OpenIFF(iff, IFFF_READ)) != 0) { + errfmt = "OpenIFF failed on %s, code %ld"; + errcode = err; + goto cleanup; + } + iff_opened = 1; + PropChunk(iff, ID_BMAP, ID_BMHD); PropChunk(iff, ID_BMAP, ID_CMAP); PropChunk(iff, ID_BMAP, ID_PDAT); StopChunk(iff, ID_BMAP, ID_PLNE); - if ((j = ParseIFF(iff, IFFPARSE_SCAN)) != 0) - panic("ParseIFF failed on %s, code %d", - filename, j); + if ((err = ParseIFF(iff, IFFPARSE_SCAN)) != 0) { + errfmt = "ParseIFF failed on %s, code %ld"; + errcode = err; + goto cleanup; + } prop = FindProp(iff, ID_BMAP, ID_BMHD); - if (!prop) - panic("No BMHD chunk in %s", filename); + if (!prop) { + errfmt = "No BMHD chunk in %s"; + goto cleanup; + } bmhd = (BitMapHeader *) prop->sp_Data; np = bmhd->nPlanes; @@ -132,18 +150,29 @@ ReadImageFile(const char *filename, struct BitMap **bmp) *bmp = MyAllocBitMap(bmhd->w, bmhd->h, np, MEMF_CHIP | MEMF_CLEAR); - if (!*bmp) - panic("Can't allocate bitmap for %s", filename); + if (!*bmp) { + errfmt = "Can't allocate bitmap for %s"; + goto cleanup; + } for (j = 0; j < np; j++) ReadChunkBytes(iff, (*bmp)->Planes[j], RASSIZE(bmhd->w, bmhd->h)); bmhds = *bmhd; - CloseIFF(iff); - Close(iff->iff_Stream); - FreeIFF(iff); + +cleanup: + if (iff_opened) + CloseIFF(iff); + if (iff && iff->iff_Stream) + Close(iff->iff_Stream); + if (iff) + FreeIFF(iff); CloseLibrary(IFFParseBase); + IFFParseBase = NULL; + + if (errfmt) + panic(errfmt, filename, errcode); return bmhds; } @@ -193,6 +222,11 @@ MyAllocBitMap(int xsize, int ysize, int depth, long mflags) bm->xsize = xsize; bm->ysize = ysize; InitBitMap(&bm->bm, depth, xsize, ysize); + /* InitBitMap does not zero Planes[]; if a later AllocRaster fails + * and MyFreeBitMap unwinds, the uninitialized entries above the + * failure would be passed to FreeRaster as garbage pointers. */ + for (j = 0; j < (int) (sizeof bm->bm.Planes / sizeof bm->bm.Planes[0]); ++j) + bm->bm.Planes[j] = NULL; for (j = 0; j < depth; ++j) { if (mflags & MEMF_CHIP) bm->bm.Planes[j] = AllocRaster(xsize, ysize);