changeset 1247:5ae2ce21c7ae

Some work on refactoring the JSSMOD loader.
author Matti Hamalainen <ccr@tnsp.org>
date Sat, 07 Mar 2015 18:57:18 +0200
parents 7bd50496c9ec
children d8324718d4b9
files minijss/jloadjss.c
diffstat 1 files changed, 139 insertions(+), 52 deletions(-) [+]
line wrap: on
line diff
--- a/minijss/jloadjss.c	Sat Mar 07 18:19:43 2015 +0200
+++ b/minijss/jloadjss.c	Sat Mar 07 18:57:18 2015 +0200
@@ -17,10 +17,6 @@
 
 
 // Short helper macros for reading data
-#define JSGETBUF(XV, XT) \
-    if (!dmf_read_str(inFile, XV, sizeof(XT))) \
-        return DMERR_OUT_OF_DATA
-
 #define JSGETBYTE(XV) \
     if (!dmf_read_byte(inFile, XV)) \
         return DMERR_OUT_OF_DATA
@@ -263,32 +259,53 @@
 #endif
 
 
-#undef JSGETBUF
-#undef JSGETBYTE
-#define JSGETBUF(XV, XT) do { \
-    if (!dmf_read_str(inFile, XV, sizeof(XT)))  \
-        JSSERROR(DMERR_OUT_OF_DATA, DMERR_OUT_OF_DATA,    \
-        "Out of data at getting " # XT " (%d bytes)\n", sizeof(XT)); \
-} while (0)
-#define JSGETBYTE(XV) if (!dmf_read_byte(inFile, XV)) return DMERR_OUT_OF_DATA
-
-
 #ifdef JM_SUP_EXT_INSTR
-static void jssCopyEnvelope(JSSEnvelope *e, JSSMODEnvelope *je)
+static int jssMODLoadEnvelope(DMResource *inFile,
+    JSSEnvelope *env, const char *name, const int ninst)
 {
+    JSSMODEnvelope jssEnv;
     int i;
 
-    e->flags   = je->flags;
-    e->npoints = je->npoints;
-    e->sustain = je->sustain;
-    e->loopS   = je->loopS;
-    e->loopE   = je->loopE;
+    // Read envelope data
+    if (!dmf_read_le16(inFile, &jssEnv.flags) ||
+        !dmf_read_le16(inFile, &jssEnv.npoints) ||
+        !dmf_read_le16(inFile, &jssEnv.sustain) ||
+        !dmf_read_le16(inFile, &jssEnv.loopS) ||
+        !dmf_read_le16(inFile, &jssEnv.loopE))
+        JSSERROR(DMERR_FREAD, DMERR_FREAD,
+        "Failed to read %s-envelope data for instrument #%d.\n",
+        name, ninst);
+
+    // Do some sanity checking
+    if (jssEnv.npoints > jsetMaxEnvPoints ||
+        jssEnv.loopS < jssEnv.loopE ||
+        jssEnv.loopS > jssEnv.npoints ||
+        jssEnv.loopE > jssEnv.npoints)
+        JSSERROR(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
+        "Invalid values in %s-envelope for instrument #%d.\n",
+        name, ninst);
 
-    for (i = 0; i < je->npoints; i++)
+    // Copy data
+    env->flags   = jssEnv.flags;
+    env->npoints = jssEnv.npoints;
+    env->sustain = jssEnv.sustain;
+    env->loopS   = jssEnv.loopS;
+    env->loopE   = jssEnv.loopE;
+
+    for (i = 0; i < jssEnv.npoints; i++)
     {
-        e->points[i].frame = je->points[i].frame;
-        e->points[i].value = je->points[i].value;
+        Uint16 frame, value;
+        if (!dmf_read_le16(inFile, &frame) ||
+            !dmf_read_le16(inFile, &value))
+            JSSERROR(DMERR_FREAD, DMERR_FREAD,
+            "Failed to read %s-envelope values (%d) for instrument #%d.\n",
+            name, i, ninst);
+
+        env->points[i].frame = frame;
+        env->points[i].value = value;
     }
+
+    return DMERR_OK;
 }
 #endif
 
@@ -297,13 +314,15 @@
 {
     JSSModule *module;
     JSSMODHeader jssH;
-    int index;
+    int index, ret = DMERR_OK;
 
     *ppModule = NULL;
 
     // Check the JSSMOD header
-    dmMemset(&jssH, 0, sizeof(jssH));
-    JSGETBUF(&jssH, JSSMODHeader);
+    if (!dmf_read_str(inFile, &jssH.idMagic, sizeof(jssH.idMagic)) ||
+        !dmf_read_byte(inFile, &jssH.idVersion))
+        JSSERROR(DMERR_FREAD, DMERR_FREAD,
+        "Failed to read JSSMOD header #1.\n");
 
     if (memcmp(jssH.idMagic, "JM", 2) != 0)
     {
@@ -324,6 +343,21 @@
         jssH.idVersion, JSSMOD_VERSION);
     }
 
+    // Read rest of the header
+    if (!dmf_read_le16(inFile, &jssH.defFlags) ||
+        !dmf_read_le16(inFile, &jssH.intVersion) ||
+        !dmf_read_le16(inFile, &jssH.norders) ||
+        !dmf_read_le16(inFile, &jssH.npatterns) ||
+        !dmf_read_le16(inFile, &jssH.nextInstruments) ||
+        !dmf_read_le16(inFile, &jssH.ninstruments) ||
+        !dmf_read_le16(inFile, &jssH.defRestartPos) ||
+        !dmf_read_byte(inFile, &jssH.nchannels) ||
+        !dmf_read_byte(inFile, &jssH.defSpeed) ||
+        !dmf_read_byte(inFile, &jssH.defTempo) ||
+        !dmf_read_byte(inFile, &jssH.patMode))
+        JSSERROR(DMERR_FREAD, DMERR_FREAD,
+        "Failed to read JSSMOD header #2.\n");
+
     if (probe)
         return DMERR_OK;
 
@@ -362,16 +396,23 @@
     // Get the orders list
     for (index = 0; index < module->norders; index++)
     {
-        Sint16 order;
-        JSGETBUF(&order, Sint16);
-        module->orderList[index] = order;
+        Uint16 tmp;
+        if (!dm_fread_le16(inFile, &tmp))
+            JSSERROR(DMERR_FREAD, DMERR_FREAD,
+            "Failed to read orders list entry #%d.\n",
+            index);
+
+        if (tmp != 0xffff && tmp > jssH.npatterns)
+            JSSERROR(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
+            "Invalid orders list entry #%d value %d > %d.\n",
+            index, tmp, jssH.npatterns);
+
     }
 
     // Parse the patterns
     for (index = 0; index < module->npatterns; index++)
     {
         JSSMODPattern jssP;
-        int result = DMERR_INVALID_DATA;
 
         // Get header and check size
         dmMemset(&jssP, 0, sizeof(jssP));
@@ -391,27 +432,27 @@
         {
 #ifdef JM_SUP_PATMODE_1
             case PATMODE_RAW_HORIZ:
-                result = jssGetPatternRawHoriz(inFile, module->patterns[index]);
+                ret = jssGetPatternRawHoriz(inFile, module->patterns[index]);
                 break;
 #endif
 #ifdef JM_SUP_PATMODE_2
             case PATMODE_COMP_HORIZ:
-                result = jssGetPatternCompHoriz(inFile, module->patterns[index]);
+                ret = jssGetPatternCompHoriz(inFile, module->patterns[index]);
                 break;
 #endif
 #ifdef JM_SUP_PATMODE_3
             case PATMODE_RAW_VERT:
-                result = jssGetPatternRawVert(inFile, module->patterns[index]);
+                ret = jssGetPatternRawVert(inFile, module->patterns[index]);
                 break;
 #endif
 #ifdef JM_SUP_PATMODE_4
             case PATMODE_COMP_VERT:
-                result = jssGetPatternCompVert(inFile, module->patterns[index]);
+                ret = jssGetPatternCompVert(inFile, module->patterns[index]);
                 break;
 #endif
 #ifdef JM_SUP_PATMODE_5
             case PATMODE_RAW_ELEM:
-                result = jssGetPatternRawVertElem(inFile, module->patterns[index]);
+                ret = jssGetPatternRawVertElem(inFile, module->patterns[index]);
                 break;
 #endif
             default:
@@ -421,9 +462,9 @@
                 break;
         }
 
-        if (result != DMERR_OK)
+        if (ret != DMERR_OK)
         {
-            JSSERROR(result, result,
+            JSSERROR(ret, ret,
             "Error in unpacking pattern #%d data.\n",
             index);
         }
@@ -437,9 +478,18 @@
         JSSExtInstrument *einst;
         int i;
 
-        dmMemset(&jssE, 0, sizeof(jssE));
-        JSGETBUF(&jssE, JSSMODExtInstrument);
-
+        // Read header data
+        if (!dmf_read_byte(inFile, &jssE.nsamples) ||
+            !dmf_read_byte(inFile, &jssE.vibratoType) ||
+            !dmf_read_le16(inFile, &jssE.vibratoSweep) ||
+            !dmf_read_le16(inFile, &jssE.vibratoDepth) ||
+            !dmf_read_le16(inFile, &jssE.vibratoRate) ||
+            !dmf_read_le16(inFile, &jssE.fadeOut))
+            JSSERROR(DMERR_FREAD, DMERR_FREAD,
+            "Failed to read ext.instrument #%d header.\n",
+            index);
+        
+        // Allocate instrument
         if ((einst = jssAllocateExtInstrument()) == NULL)
         {
             JSSERROR(DMERR_MALLOC, DMERR_MALLOC,
@@ -455,14 +505,33 @@
         einst->vibratoRate  = jssE.vibratoRate;
         einst->fadeOut      = jssE.fadeOut;
 
+        // Read and somewhat validate sNumForNotes
         for (i = 0; i < jsetNNotes; i++)
         {
-            int snum = jssE.sNumForNotes[i];
-            einst->sNumForNotes[i] = (snum > 0) ? snum - 1 : jsetNotSet;
+            int snum;
+            Uint32 tmp;
+
+            if (!dmf_read_le32(inFile, &tmp))
+            {
+                JSSERROR(DMERR_FREAD, DMERR_FREAD,
+                "Failed to read ext.instrument #%d sNumForNotes[%d].\n",
+                index, i);
+            }
+
+            einst->sNumForNotes[i] = snum = (tmp > 0) ? ((int) tmp - 1) : jsetNotSet;
+
+            if (snum != jsetNotSet && snum > module->ninstruments)
+            {
+                JSSERROR(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
+                "Ext.instrument #%d has invalid sNumForNotes[%d] = %d > %d max.\n",
+                index, i, snum, module->ninstruments);
+            }
         }
 
-        jssCopyEnvelope(&(einst->volumeEnv), &jssE.volumeEnv);
-        jssCopyEnvelope(&(einst->panningEnv), &jssE.panningEnv);
+        // Read and validate envelopes
+        if ((ret = jssMODLoadEnvelope(inFile, &einst->volumeEnv, "volume", index)) != DMERR_OK ||
+            (ret = jssMODLoadEnvelope(inFile, &einst->panningEnv, "panning", index)) != DMERR_OK)
+            return ret;
     }
 
 #ifdef JM_SUP_INSTR
@@ -472,27 +541,45 @@
         JSSMODInstrument jssI;
         JSSInstrument *inst;
 
-        dmMemset(&jssI, 0, sizeof(jssI));
-        JSGETBUF(&jssI, JSSMODInstrument);
-
-        if ((inst = jssAllocateInstrument()) == NULL)
+        // Read header data
+        if (!dmf_read_le32(inFile, &jssI.size) ||
+            !dmf_read_le32(inFile, &jssI.loopS) ||
+            !dmf_read_le32(inFile, &jssI.loopE) ||
+            !dmf_read_le16(inFile, &jssI.flags) ||
+            !dmf_read_le16(inFile, &jssI.C4BaseSpeed) ||
+            !dmf_read_le16(inFile, &jssI.ERelNote) ||
+            !dmf_read_le16(inFile, &jssI.EFineTune) ||
+            !dmf_read_le16(inFile, &jssI.EPanning) ||
+            !dmf_read_byte(inFile, &jssI.volume) ||
+            !dmf_read_byte(inFile, &jssI.convFlags))
         {
-            JSSERROR(DMERR_MALLOC, DMERR_MALLOC,
-            "Could not allocate sample instrument structure #%d\n", index);
+            JSSERROR(DMERR_FREAD, DMERR_FREAD,
+            "Failed to read sample instrument #%d header.\n",
+            index);
         }
 
-        module->instruments[index] = inst;
+        // Validate what we can
+        if (jssI.loopS > jssI.size ||
+            jssI.loopE > jssI.size ||
+            jssI.loopE > jssI.loopS)
+        {
+            JSSERROR(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
+            "Invalid or corrupted sample instrument #%d.\n",
+            index);
+        }
+
+        // Allocate instrument
 
         // Copy data
         inst->size          = jssI.size;
         inst->loopS         = jssI.loopS;
         inst->loopE         = jssI.loopE;
-        inst->volume        = jssI.volume;
         inst->flags         = jssI.flags;
         inst->C4BaseSpeed   = jssI.C4BaseSpeed;
         inst->ERelNote      = jssI.ERelNote;
         inst->EFineTune     = jssI.EFineTune;
         inst->EPanning      = jssI.EPanning;
+        inst->volume        = jssI.volume;
         inst->convFlags     = jssI.convFlags;
     }