# HG changeset patch
# User Matti Hamalainen <ccr@tnsp.org>
# Date 1578593501 -7200
# Node ID 2dbbc1c91231e87933de16cd2d6a2e885099139b
# Parent  68efae89c0341345d992a13888f6933842f856d3
Refactor error handling in dmC64DecodeGenericBMP() and
dmC64EncodeGenericBMP() by adding helper functions dmC64EncDecError() and
dmC64EncDecErrorMsg() and using them through out.

Also various printf() format specifier fixes.

diff -r 68efae89c034 -r 2dbbc1c91231 tools/lib64gfx.c
--- a/tools/lib64gfx.c	Thu Jan 09 19:59:00 2020 +0200
+++ b/tools/lib64gfx.c	Thu Jan 09 20:11:41 2020 +0200
@@ -869,8 +869,51 @@
 }
 
 
+typedef struct
+{
+    int opn;
+    const DMC64EncDecOp *op;
+    size_t size;
+    const DMGrowBuf *buf;
+    const char *subjname;
+} DMC64EncDecCtx;
+
+
+static void dmC64EncDecErrorMsg(DMC64EncDecCtx *ctx, const char *msg)
+{
+    dmErrorMsg(
+        "%s in op #%d, subject='%s', "
+        "offs=%d ($%04x), bank=%d, "
+        "size=%" DM_PRIu_SIZE_T " ($%04" DM_PRIx_SIZE_T ") "
+        "@ %" DM_PRIu_SIZE_T " ($%04" DM_PRIx_SIZE_T ")\n",
+        msg, ctx->opn, ctx->subjname,
+        ctx->op->offs, ctx->op->offs,
+        ctx->op->bank,
+        ctx->size, ctx->size,
+        ctx->buf->len, ctx->buf->len);
+}
+
+
+DM_ATTR_PRINTF_FMT(3, 4)
+static int dmC64EncDecError(DMC64EncDecCtx *ctx, const int res, const char *fmt, ...)
+{
+    char *msg;
+    va_list ap;
+
+    va_start(ap, fmt);
+    msg = dm_strdup_vprintf(fmt, ap);
+    va_end(ap);
+
+    dmC64EncDecErrorMsg(ctx, msg);
+
+    dmFree(msg);
+    return res;
+}
+
+
 int dmC64DecodeGenericBMP(DMC64Image *img, const DMGrowBuf *buf, const DMC64ImageFormat *fmt)
 {
+    DMC64EncDecCtx ctx;
     int res = DMERR_OK;
 
     if (buf == NULL || buf->data == NULL || img == NULL || fmt == NULL)
@@ -879,49 +922,46 @@
     dmC64SetupImageData(img, fmt);
 
     // Perform decoding
-    for (int i = 0; i < D64_MAX_ENCDEC_OPS; i++)
+    for (ctx.opn = 0; ctx.opn < D64_MAX_ENCDEC_OPS; ctx.opn++)
     {
-        const DMC64EncDecOp *op = fmtGetEncDecOp(fmt, i);
+        DMC64MemBlock *blk = NULL;
         const Uint8 *src;
-        DMC64MemBlock *blk = NULL;
-        const char *subjname = dmC64GetOpSubjectName(op->subject);
-        size_t size;
         Uint8 value;
 
+        ctx.op = fmtGetEncDecOp(fmt, ctx.opn);
+        ctx.subjname = dmC64GetOpSubjectName(ctx.op->subject);
+
         // Check for last operator
-        if (op->type == DO_LAST)
+        if (ctx.op->type == DO_LAST)
             break;
 
         // Check operation validity
-        if ((res = dmC64SanityCheckEncDecOp(i, op, img)) != DMERR_OK)
+        if ((res = dmC64SanityCheckEncDecOp(ctx.opn, ctx.op, img)) != DMERR_OK)
             return res;
 
         // Check flags
-        if ((op->flags & DF_DECODE) == 0)
+        if ((ctx.op->flags & DF_DECODE) == 0)
             continue;
 
         // Is the operation inside the bounds?
-        size = dmC64GetOpSubjectSize(op, fmt->format);
-        if (op->type == DO_COPY && op->offs + size > buf->len + 1)
+        ctx.size = dmC64GetOpSubjectSize(ctx.op, fmt->format);
+        if (ctx.op->type == DO_COPY && ctx.op->offs + ctx.size > buf->len + 1)
         {
-            return dmError(DMERR_INVALID_DATA,
-                "Decode SRC out of bounds, op #%d type=%s, subj=%s, offs=%d ($%04x), "
-                "bank=%d, size=%d ($%04x) @ %d ($%04x)\n",
-                i, dmC64GetOpType(op->type), subjname, op->offs, op->offs, op->bank,
-                size, size, buf->len, buf->len);
+            return dmC64EncDecError(&ctx, DMERR_INVALID_DATA,
+                "Decode SRC out of bounds");
         }
 
-        src = buf->data + op->offs;
+        src = buf->data + ctx.op->offs;
 
         // Perform operation
-        switch (op->type)
+        switch (ctx.op->type)
         {
             case DO_COPY:
             case DO_SET_MEM:
             case DO_SET_MEM_HI:
             case DO_SET_MEM_LO:
             case DO_SET_OP:
-                switch (op->subject)
+                switch (ctx.op->subject)
                 {
                     case DS_COLOR_RAM:
                     case DS_SCREEN_RAM:
@@ -929,44 +969,40 @@
                     case DS_CHAR_DATA:
                     case DS_EXTRA_DATA:
                         // XXX BZZZT .. a nasty cast here
-                        blk = (DMC64MemBlock *) dmC64GetOpMemBlock(img, op->subject, op->bank);
+                        blk = (DMC64MemBlock *) dmC64GetOpMemBlock(img, ctx.op->subject, ctx.op->bank);
 
-                        if ((dmC64MemBlockReAlloc(blk, op->blkoffs + size)) != DMERR_OK)
+                        if ((dmC64MemBlockReAlloc(blk, ctx.op->blkoffs + ctx.size)) != DMERR_OK)
                         {
-                            return dmError(DMERR_MALLOC,
-                                "Could not allocate '%s' block! "
-                                "op #%d, offs=%d ($%04x), bank=%d, size=%d ($%04x) @ %d ($%04x)\n",
-                                subjname, i, op->offs, op->offs, op->bank,
-                                op->blkoffs + size, op->blkoffs + size, buf->len, buf->len);
+                            return dmC64EncDecError(&ctx, DMERR_MALLOC,
+                                "Could not allocate '%s' block",
+                                ctx.subjname);
                         }
-                        switch (op->type)
+                        switch (ctx.op->type)
                         {
                             case DO_COPY:
-                                memcpy(blk->data + op->blkoffs, src, size);
+                                memcpy(blk->data + ctx.op->blkoffs, src, ctx.size);
                                 break;
 
                             case DO_SET_MEM:
-                                dmMemset(blk->data + op->blkoffs, *src, size);
+                                dmMemset(blk->data + ctx.op->blkoffs, *src, ctx.size);
                                 break;
 
                             case DO_SET_MEM_HI:
-                                dmMemset(blk->data + op->blkoffs, (*src >> 4) & 0x0f, size);
+                                dmMemset(blk->data + ctx.op->blkoffs, (*src >> 4) & 0x0f, ctx.size);
                                 break;
 
                             case DO_SET_MEM_LO:
-                                dmMemset(blk->data + op->blkoffs, *src & 0x0f, size);
+                                dmMemset(blk->data + ctx.op->blkoffs, *src & 0x0f, ctx.size);
                                 break;
 
                             case DO_SET_OP:
-                                dmMemset(blk->data + op->blkoffs, op->offs, size);
+                                dmMemset(blk->data + ctx.op->blkoffs, ctx.op->offs, ctx.size);
                                 break;
 
                             default:
-                                return dmError(DMERR_INTERNAL,
-                                    "Unhandled op type %s in "
-                                    "op #%d, offs=%d ($%04x), bank=%d, size=%d ($%04x) @ %d ($%04x)\n",
-                                    dmC64GetOpType(op->type), i, op->offs, op->offs, op->bank,
-                                    size, size, buf->len, buf->len);
+                                return dmC64EncDecError(&ctx, DMERR_INTERNAL,
+                                    "Unhandled op type '%s'",
+                                    dmC64GetOpType(ctx.op->type));
                         }
                         break;
 
@@ -977,7 +1013,7 @@
                     case DS_D023:
                     case DS_D024:
                     case DS_EXTRA_INFO:
-                        switch (op->type)
+                        switch (ctx.op->type)
                         {
                             case DO_COPY:
                             case DO_SET_MEM:
@@ -985,7 +1021,7 @@
                                 break;
 
                             case DO_SET_OP:
-                                value = op->offs;
+                                value = ctx.op->offs;
                                 break;
 
                             case DO_SET_MEM_HI:
@@ -997,13 +1033,11 @@
                                 break;
 
                             default:
-                                return dmError(DMERR_INTERNAL,
-                                    "Unhandled op type %s in "
-                                    "op #%d, offs=%d ($%04x), bank=%d, size=%d ($%04x) @ %d ($%04x)\n",
-                                    dmC64GetOpType(op->type), i, op->offs, op->offs,
-                                    op->bank, size, size, buf->len, buf->len);
+                                return dmC64EncDecError(&ctx, DMERR_INTERNAL,
+                                    "Unhandled op type '%s'",
+                                    dmC64GetOpType(ctx.op->type));
                         }
-                        switch (op->subject)
+                        switch (ctx.op->subject)
                         {
                             case DS_D020: img->d020 = value; break;
                             case DS_BGCOL:
@@ -1011,20 +1045,19 @@
                             case DS_D022: img->d022 = value; break;
                             case DS_D023: img->d023 = value; break;
                             case DS_D024: img->d024 = value; break;
-                            case DS_EXTRA_INFO: img->extraInfo[op->blkoffs] = value; break;
+                            case DS_EXTRA_INFO: img->extraInfo[ctx.op->blkoffs] = value; break;
                         }
                         break;
 
                     default:
-                        return dmError(DMERR_INTERNAL,
-                            "Unhandled subject %d in "
-                            "op #%d, offs=%d ($%04x), bank=%d, size=%d ($%04x) @ %d ($%04x)\n",
-                            op->subject, i, op->offs, op->offs, op->bank, size, size, buf->len, buf->len);
+                        return dmC64EncDecError(&ctx, DMERR_INTERNAL,
+                            "Unhandled subject '%s'",
+                            ctx.subjname);
                 }
                 break;
 
             case DO_CHAR_CFG:
-                switch (op->subject)
+                switch (ctx.op->subject)
                 {
                     case D64_CHCFG_SCREEN:
                         break;
@@ -1038,21 +1071,18 @@
                         break;
 
                     default:
-                        return dmError(DMERR_INTERNAL,
-                            "Unhandled DO_CHAR_CFG mode %d in ",
-                            "op #%d, bank=%d, size=%d ($%04x) @ %d ($%04x)\n",
-                            op->subject, i, op->bank, size, size, buf->len, buf->len);
+                        return dmC64EncDecError(&ctx, DMERR_INTERNAL,
+                            "Unhandled DO_CHAR_CFG mode %d",
+                            ctx.op->subject);
                 }
                 break;
 
             case DO_FUNC:
-                if (op->decFunction != NULL &&
-                    (res = op->decFunction(op, img, buf, fmt->format)) != DMERR_OK)
+                if (ctx.op->decFunction != NULL &&
+                    (res = ctx.op->decFunction(ctx.op, img, buf, fmt->format)) != DMERR_OK)
                 {
-                    return dmError(res,
-                        "Decode op custom function failed: op #%d, "
-                        "offs=%d ($%04x), bank=%d, size=%d ($%04x) @ %d ($%04x)\n",
-                        i, op->offs, op->offs, op->bank, size, size, buf->len, buf->len);
+                    return dmC64EncDecError(&ctx, res,
+                        "Decode op custom function failed");
                 }
                 break;
         }
@@ -1073,6 +1103,7 @@
 
 int dmC64EncodeGenericBMP(const BOOL allocate, DMGrowBuf *buf, const DMC64Image *img, const DMC64ImageFormat *fmt)
 {
+    DMC64EncDecCtx ctx;
     int res = DMERR_OK;
 
     if (img == NULL || fmt == NULL)
@@ -1081,88 +1112,82 @@
     // Allocate the output buffer if requested
     if (allocate && (res = dmGrowBufAlloc(buf, BUF_SIZE_INITIAL, BUF_SIZE_GROW)) != DMERR_OK)
     {
-        dmError(res,
-            "Could not allocate %d bytes of memory for C64 image encoding buffer.\n",
+        return dmError(res,
+            "Could not allocate %" DM_PRIu_SIZE_T " bytes of memory for C64 image encoding buffer.\n",
             fmt->size);
-        goto out;
     }
 
     if (buf->backwards)
     {
-        dmError(DMERR_INVALID_DATA,
+        return dmError(DMERR_INVALID_DATA,
             "Buffer specified for dmC64EncodeGenericBMP() is in backwards mode, which is not supported.\n");
-        goto out;
     }
 
     // Perform encoding
-    for (int i = 0; i < D64_MAX_ENCDEC_OPS; i++)
+    for (ctx.opn = 0; ctx.opn < D64_MAX_ENCDEC_OPS; ctx.opn++)
     {
-        const DMC64EncDecOp *op = fmtGetEncDecOp(fmt, i);
-        size_t size, chksize;
         const DMC64MemBlock *blk = NULL;
-        const char *subjname = dmC64GetOpSubjectName(op->subject);
+        size_t chksize;
         Uint8 value;
 
+        ctx.op = fmtGetEncDecOp(fmt, ctx.opn);
+        ctx.subjname = dmC64GetOpSubjectName(ctx.op->subject);
+
         // Check for last operator
-        if (op->type == DO_LAST)
+        if (ctx.op->type == DO_LAST)
             break;
 
         // Check operation validity
-        if ((res = dmC64SanityCheckEncDecOp(i, op, img)) != DMERR_OK)
-            goto out;
+        if ((res = dmC64SanityCheckEncDecOp(ctx.opn, ctx.op, img)) != DMERR_OK)
+            return res;
 
         // Check flags
-        if ((op->flags & DF_ENCODE) == 0)
+        if ((ctx.op->flags & DF_ENCODE) == 0)
             continue;
 
         // Do we need to reallocate some more space?
-        size = dmC64GetOpSubjectSize(op, fmt->format);
-        chksize = buf->offs + op->offs + size;
+        ctx.size = dmC64GetOpSubjectSize(ctx.op, fmt->format);
+        chksize = buf->offs + ctx.op->offs + ctx.size;
         if (!dmGrowBufCheckGrow(buf, chksize))
         {
-            res = dmError(DMERR_MALLOC,
-                "Could not re-allocate %d bytes of memory for C64 image encoding buffer.\n",
+            return dmError(DMERR_MALLOC,
+                "Could not re-allocate %" DM_PRIu_SIZE_T " bytes of memory for C64 image encoding buffer.\n",
                 chksize);
-            goto out;
         }
 
         // Perform operation
-        Uint8 *dst = buf->data + buf->offs + op->offs;
-        switch (op->type)
+        Uint8 *dst = buf->data + buf->offs + ctx.op->offs;
+        switch (ctx.op->type)
         {
             case DO_COPY:
             case DO_SET_MEM:
             case DO_SET_MEM_HI:
             case DO_SET_MEM_LO:
             case DO_SET_OP:
-                switch (op->subject)
+                switch (ctx.op->subject)
                 {
                     case DS_COLOR_RAM:
                     case DS_SCREEN_RAM:
                     case DS_BITMAP_RAM:
                     case DS_CHAR_DATA:
                     case DS_EXTRA_DATA:
-                        blk = dmC64GetOpMemBlock(img, op->subject, op->bank);
-                        switch (op->type)
+                        blk = dmC64GetOpMemBlock(img, ctx.op->subject, ctx.op->bank);
+                        switch (ctx.op->type)
                         {
                             case DO_COPY:
                                 if (blk->data == NULL)
                                 {
-                                    res = dmError(DMERR_NULLPTR,
-                                        "'%s' block is NULL in "
-                                        "op #%d, offs=%d ($%04x), bank=%d, size=%d ($%04x) @ %d ($%04x)\n",
-                                        subjname, i, op->offs, op->offs, op->bank, size, size, buf->len, buf->len);
-                                    goto out;
+                                    return dmC64EncDecError(&ctx, DMERR_NULLPTR,
+                                        "'%s' block is NULL",
+                                        ctx.subjname);
                                 }
-                                if (op->blkoffs + size > blk->size)
+                                if (ctx.op->blkoffs + ctx.size > blk->size)
                                 {
-                                    res = dmError(DMERR_INTERNAL,
-                                        "'%s' size mismatch %d <> %d in "
-                                        "op #%d, offs=%d ($%04x), bank=%d, offs2=%d ($%02x), size=%d ($%04x)\n",
-                                        subjname, op->blkoffs + size, blk->size, i, op->offs, op->offs, op->bank, op->blkoffs, op->blkoffs, size, size);
-                                    goto out;
+                                    return dmC64EncDecError(&ctx, DMERR_INTERNAL,
+                                        "'%s' size mismatch %" DM_PRIu_SIZE_T " <> %" DM_PRIu_SIZE_T,
+                                        ctx.subjname, ctx.op->blkoffs + ctx.size, blk->size);
                                 }
-                                memcpy(dst, blk->data + op->blkoffs, size);
+                                memcpy(dst, blk->data + ctx.op->blkoffs, ctx.size);
                                 break;
 
                             case DO_SET_MEM:
@@ -1173,10 +1198,9 @@
                                 break;
 
                             default:
-                                return dmError(DMERR_INTERNAL,
-                                    "Unhandled op type %s in "
-                                    "op #%d, offs=%d ($%04x), bank=%d, size=%d ($%04x) @ %d ($%04x)\n",
-                                    dmC64GetOpType(op->type), i, op->offs, op->offs, op->bank, size, size, buf->len, buf->len);
+                                return dmC64EncDecError(&ctx, DMERR_INTERNAL,
+                                    "Unhandled op type '%s'",
+                                    dmC64GetOpType(ctx.op->type));
                         }
                         break;
 
@@ -1187,7 +1211,7 @@
                     case DS_D023:
                     case DS_D024:
                     case DS_EXTRA_INFO:
-                        switch (op->subject)
+                        switch (ctx.op->subject)
                         {
                             case DS_D020: value = img->d020; break;
                             case DS_BGCOL:
@@ -1195,9 +1219,9 @@
                             case DS_D022: value = img->d022; break;
                             case DS_D023: value = img->d023; break;
                             case DS_D024: value = img->d024; break;
-                            case DS_EXTRA_INFO: value = img->extraInfo[op->blkoffs]; break;
+                            case DS_EXTRA_INFO: value = img->extraInfo[ctx.op->blkoffs]; break;
                         }
-                        switch (op->type)
+                        switch (ctx.op->type)
                         {
                             case DO_COPY:
                             case DO_SET_MEM:
@@ -1218,39 +1242,30 @@
                                 break;
 
                             default:
-                                return dmError(DMERR_INTERNAL,
-                                    "Unhandled op type %s in "
-                                    "op #%d, offs=%d ($%04x), bank=%d, size=%d ($%04x) @ %d ($%04x)\n",
-                                    dmC64GetOpType(op->type), i, op->offs, op->offs, op->bank, size, size, buf->len, buf->len);
+                                return dmC64EncDecError(&ctx, DMERR_INTERNAL,
+                                    "Unhandled op type '%s'",
+                                    dmC64GetOpType(ctx.op->type));
                         }
                         break;
 
                     default:
-                        return dmError(DMERR_INTERNAL,
-                            "Unhandled subject '%s' in "
-                            "op #%d, offs=%d ($%04x), bank=%d, size=%d ($%04x) @ %d ($%04x)\n",
-                            subjname, i, op->offs, op->offs, op->bank, size, size, buf->len, buf->len);
+                        return dmC64EncDecError(&ctx, DMERR_INTERNAL,
+                            "Unhandled subject '%s'", ctx.subjname);
                 }
                 break;
 
             case DO_FUNC:
-                if (op->encFunction != NULL &&
-                    (res = op->encFunction(op, buf, img, fmt->format)) != DMERR_OK)
+                if (ctx.op->encFunction != NULL &&
+                    (res = ctx.op->encFunction(ctx.op, buf, img, fmt->format)) != DMERR_OK)
                 {
-                    dmErrorMsg(
-                        "Encode op custom function failed: op #%d, "
-                        "offs=%d ($%04x), bank=%d, size=%d ($%04x) @ %d ($%04x)\n",
-                        i, op->offs, op->offs, op->bank, size, size, buf->len, buf->len);
-                    goto out;
+                    return dmC64EncDecError(&ctx, res,
+                        "Encode op custom function failed");
                 }
                 break;
         }
     }
 
-    res = DMERR_OK;
-
-out:
-    return res;
+    return DMERR_OK;
 }
 
 
@@ -1577,7 +1592,7 @@
     if ((res = dmC64MemBlockAlloc(blk, size)) != DMERR_OK)
     {
         return dmError(res,
-            "Could not allocate '%s' block with size %d bytes.\n",
+            "Could not allocate '%s' block with size %" DM_PRIu_SIZE_T " bytes.\n",
             subjname, size);
     }