changeset 778:a4cf7716ba58

Clean up the XM module loader a bit, add more error checking.
author Matti Hamalainen <ccr@tnsp.org>
date Fri, 24 May 2013 01:34:00 +0300
parents ed60a7ee3ebb
children 954b1b392c8b
files minijss/jloadxm.c
diffstat 1 files changed, 122 insertions(+), 97 deletions(-) [+]
line wrap: on
line diff
--- a/minijss/jloadxm.c	Fri May 24 01:19:11 2013 +0300
+++ b/minijss/jloadxm.c	Fri May 24 01:34:00 2013 +0300
@@ -85,9 +85,9 @@
 {
     Uint32 size, loopS, loopL;
     Uint8 volume;
-    int fineTune;
+    Sint8 fineTune;
     Uint8 type, panning;
-    int relNote;
+    Sint8 relNote;
     Uint8 ARESERVED;
     char sampleName[22];
 } XMSample;
@@ -104,12 +104,12 @@
 
 /* Unpack XM-format pattern from file-stream into JSS-pattern structure
  */
-#define JSGETBYTE(XV) do {                          \
-    size--;                                         \
-    if (size < 0)                                   \
-    JSSERROR(DMERR_OUT_OF_DATA, DMERR_OUT_OF_DATA,  \
-    "Unexpected end of packed pattern data.\n");    \
-    XV = dmfgetc(inFile);                           \
+#define JSGETBYTE(XV) do {                              \
+    size--;                                             \
+    if (size < 0)                                       \
+        JSSERROR(DMERR_OUT_OF_DATA, DMERR_OUT_OF_DATA,  \
+        "Unexpected end of packed pattern data.\n");    \
+    XV = dmfgetc(inFile);                               \
 } while (0)
 
 
@@ -203,7 +203,7 @@
     {
         // Some data left unparsed
         JSSWARNING(DMERR_EXTRA_DATA, DMERR_EXTRA_DATA,
-        "Unparsed data after pattern (%i bytes), possibly broken file.\n", size);
+        "Unparsed data after pattern (%d bytes), possibly broken file.\n", size);
     }
 
     return DMERR_OK;
@@ -245,14 +245,14 @@
         if (s->npoints > XM_MaxEnvPoints)
         {
             JSSWARNING(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
-            "Inst#%i/%s-env: nPoints > MAX, possibly broken file.\n", instr, e);
+            "Inst#%d/%s-env: nPoints > MAX, possibly broken file.\n", instr, e);
             s->npoints = XM_MaxEnvPoints;
         }
 
         if ((d->flags & jenvfSustain) && s->sustain > s->npoints)
         {
             JSSWARNING(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
-            "Inst#%i/%s-env: iSustain > nPoints (%i > %i), possibly broken file.\n",
+            "Inst#%d/%s-env: iSustain > nPoints (%d > %d), possibly broken file.\n",
             instr, e, s->sustain, s->npoints);
             s->sustain = s->npoints;
         }
@@ -260,7 +260,7 @@
         if ((d->flags & jenvfLooped) && s->loopE > s->npoints)
         {
             JSSWARNING(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
-            "Inst#%i/%s-env: loopE > nPoints (%i > %i), possibly broken file.\n",
+            "Inst#%d/%s-env: loopE > nPoints (%d > %d), possibly broken file.\n",
             instr, e, s->loopE, s->npoints);
             s->loopE = s->npoints;
         }
@@ -268,7 +268,7 @@
         if ((d->flags & jenvfLooped) && s->loopS > s->loopE)
         {
             JSSWARNING(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
-            "Inst#%i/%s-env: loopS > loopE (%i > %i), possibly broken file.\n",
+            "Inst#%d/%s-env: loopS > loopE (%d > %d), possibly broken file.\n",
             instr, e, s->loopS, s->loopE);
             s->loopS = 0;
         }
@@ -287,10 +287,11 @@
 
     // Get instrument header #1
     pos = dmftell(inFile);
-    dmf_read_le32(inFile, &xmI1.headSize);
-    dmf_read_str(inFile, &xmI1.instName, sizeof(xmI1.instName));
-    xmI1.instType = dmfgetc(inFile);
-    dmf_read_le16(inFile, &xmI1.nsamples);
+    if (!dmf_read_le32(inFile, &xmI1.headSize) ||
+        !dmf_read_str(inFile, &xmI1.instName, sizeof(xmI1.instName)) ||
+        !dmf_read_byte(inFile, &xmI1.instType) ||
+        !dmf_read_le16(inFile, &xmI1.nsamples))
+        return DMERR_FREAD;
 
     // If there are samples, there is header #2
     if (xmI1.nsamples > 0)
@@ -305,14 +306,18 @@
         if ((pEInst = jssAllocateExtInstrument()) == NULL)
         {
             JSSERROR(DMERR_MALLOC, DMERR_MALLOC,
-            "Could not allocate extended instrument structure #%i\n", ninst);
+            "Could not allocate extended instrument structure #%d\n", ninst);
         }
 
         module->extInstruments[ninst] = pEInst;
 
         // Get instrument header #2
-        dmf_read_le32(inFile, &xmI2.headSize);
-        dmf_read_str(inFile, &xmI2.sNumForNotes, sizeof(xmI2.sNumForNotes));
+        if (!dmf_read_le32(inFile, &xmI2.headSize) ||
+            !dmf_read_str(inFile, &xmI2.sNumForNotes, sizeof(xmI2.sNumForNotes)))
+        {
+            JSSERROR(DMERR_FREAD, DMERR_FREAD,
+            "Could not read secondary instrument header #1 for #%d.\n", ninst);
+        }
 
         for (i = 0; i < XM_MaxEnvPoints; i++)
         {
@@ -326,27 +331,31 @@
             dmf_read_le16(inFile, &xmI2.panningEnv.points[i].value);
         }
 
-        xmI2.volumeEnv.npoints = dmfgetc(inFile);
-        xmI2.panningEnv.npoints = dmfgetc(inFile);
+        if (!dmf_read_byte(inFile, &xmI2.volumeEnv.npoints) ||
+            !dmf_read_byte(inFile, &xmI2.panningEnv.npoints) ||
 
-        xmI2.volumeEnv.sustain = dmfgetc(inFile);
-        xmI2.volumeEnv.loopS = dmfgetc(inFile);
-        xmI2.volumeEnv.loopE = dmfgetc(inFile);
+            !dmf_read_byte(inFile, &xmI2.volumeEnv.sustain) ||
+            !dmf_read_byte(inFile, &xmI2.volumeEnv.loopS) ||
+            !dmf_read_byte(inFile, &xmI2.volumeEnv.loopE) ||
 
-        xmI2.panningEnv.sustain = dmfgetc(inFile);
-        xmI2.panningEnv.loopS = dmfgetc(inFile);
-        xmI2.panningEnv.loopE = dmfgetc(inFile);
+            !dmf_read_byte(inFile, &xmI2.panningEnv.sustain) ||
+            !dmf_read_byte(inFile, &xmI2.panningEnv.loopS) ||
+            !dmf_read_byte(inFile, &xmI2.panningEnv.loopE) ||
 
-        xmI2.volumeEnv.flags = dmfgetc(inFile);
-        xmI2.panningEnv.flags = dmfgetc(inFile);
+            !dmf_read_byte(inFile, &xmI2.volumeEnv.flags) ||
+            !dmf_read_byte(inFile, &xmI2.panningEnv.flags) ||
 
-        xmI2.vibratoType = dmfgetc(inFile);
-        xmI2.vibratoSweep = dmfgetc(inFile);
-        xmI2.vibratoDepth = dmfgetc(inFile);
-        xmI2.vibratoRate = dmfgetc(inFile);
+            !dmf_read_byte(inFile, &xmI2.vibratoType) ||
+            !dmf_read_byte(inFile, &xmI2.vibratoSweep) ||
+            !dmf_read_byte(inFile, &xmI2.vibratoDepth) ||
+            !dmf_read_byte(inFile, &xmI2.vibratoRate) ||
 
-        dmf_read_le16(inFile, &xmI2.fadeOut);
-        dmf_read_le16(inFile, &xmI2.ARESERVED);
+            !dmf_read_le16(inFile, &xmI2.fadeOut) ||
+            !dmf_read_le16(inFile, &xmI2.ARESERVED))
+        {
+            JSSERROR(DMERR_FREAD, DMERR_FREAD,
+            "Could not read secondary instrument header #2 for #%d.\n", ninst);
+        }
 
         // Skip the extra data after header #2
         remainder = xmI1.headSize - (dmftell(inFile) - pos);
@@ -377,9 +386,9 @@
         }
         pEInst->vibratoSweep = xmI2.vibratoSweep;
         pEInst->vibratoDepth = xmI2.vibratoDepth;
-        pEInst->vibratoRate = xmI2.vibratoRate;
-        pEInst->fadeOut = xmI2.fadeOut;
-        pEInst->nsamples = xmI1.nsamples;
+        pEInst->vibratoRate  = xmI2.vibratoRate;
+        pEInst->fadeOut      = xmI2.fadeOut;
+        pEInst->nsamples     = xmI1.nsamples;
 
         // Initialize the SNumForNotes conversion table
         for (i = 0; i < XM_MaxInstruments; i++)
@@ -391,21 +400,26 @@
             XMSample xmS;
 
             // Read header data
-            dmf_read_le32(inFile, &xmS.size);
-            dmf_read_le32(inFile, &xmS.loopS);
-            dmf_read_le32(inFile, &xmS.loopL);
-            xmS.volume = dmfgetc(inFile);
-            xmS.fineTune = (signed char) dmfgetc(inFile);
-            xmS.type = dmfgetc(inFile);
-            xmS.panning = dmfgetc(inFile);
-            xmS.relNote = (signed char) dmfgetc(inFile);
-            xmS.ARESERVED = dmfgetc(inFile);
-            dmf_read_str(inFile, &xmS.sampleName, sizeof(xmS.sampleName));
+            if (!dmf_read_le32(inFile, &xmS.size) ||
+                !dmf_read_le32(inFile, &xmS.loopS) ||
+                !dmf_read_le32(inFile, &xmS.loopL) ||
+                !dmf_read_byte(inFile, &xmS.volume) ||
+                !dmf_read_byte(inFile, (Uint8 *) &xmS.fineTune) ||
+                !dmf_read_byte(inFile, &xmS.type) ||
+                !dmf_read_byte(inFile, &xmS.panning) ||
+                !dmf_read_byte(inFile, (Uint8 *) &xmS.relNote) ||
+                !dmf_read_byte(inFile, &xmS.ARESERVED) ||
+                !dmf_read_str(inFile, &xmS.sampleName, sizeof(xmS.sampleName)))
+            {
+                JSSERROR(DMERR_FREAD, DMERR_FREAD,
+                "Error reading instrument sample header #%d/%d [%d]",
+                ninst, nsample, module->ninstruments);
+            }
 
             if (xmS.size > 0)
             {
                 // Allocate sample instrument
-                JSSDEBUG("Allocating sample #%i/%i [%i]\n",
+                JSSDEBUG("Allocating sample #%d/%d [%d]\n",
                 ninst, nsample, module->ninstruments);
 
                 xmConvTable[nsample] = module->ninstruments;
@@ -413,7 +427,7 @@
                 if (pInst == NULL)
                 {
                     JSSERROR(DMERR_MALLOC, DMERR_MALLOC,
-                    "Could not allocate sample #%i/%i [%i]\n",
+                    "Could not allocate sample #%d/%d [%d]\n",
                     ninst, nsample, module->ninstruments);
                 }
                 module->ninstruments++;
@@ -427,14 +441,14 @@
                 if (xmS.volume > XM_MaxSampleVolume)
                 {
                     JSSWARNING(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
-                           "Samp #%i/%i: volume > MAX\n", ninst, nsample);
+                           "Samp #%d/%d: volume > MAX\n", ninst, nsample);
                     xmS.volume = XM_MaxSampleVolume;
                 }
 
-                pInst->volume = xmS.volume;
-                pInst->ERelNote = xmS.relNote;
+                pInst->volume    = xmS.volume;
+                pInst->ERelNote  = xmS.relNote;
                 pInst->EFineTune = xmS.fineTune;
-                pInst->EPanning = xmS.panning;
+                pInst->EPanning  = xmS.panning;
 #ifndef JSS_LIGHT
                 pInst->desc = jssASCIItoStr(xmS.sampleName, 0, sizeof(xmS.sampleName));
 #endif
@@ -448,7 +462,7 @@
                     default:
                         pInst->flags = 0;
                         JSSWARNING(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
-                        "Samp #%i/%i: Invalid sample type 0x%x\n",
+                        "Samp #%d/%d: Invalid sample type 0x%x\n",
                         ninst, nsample, xmS.type);
                         break;
                 }
@@ -458,14 +472,14 @@
                     // 16-bit sample
                     JSFSET(pInst->flags, jsf16bit);
 
-                    pInst->size = xmS.size / sizeof(Uint16);
+                    pInst->size  = xmS.size / sizeof(Uint16);
                     pInst->loopS = xmS.loopS / sizeof(Uint16);
                     pInst->loopE = ((xmS.loopS + xmS.loopL) / sizeof(Uint16));
                 }
                 else
                 {
                     // 8-bit sample
-                    pInst->size = xmS.size;
+                    pInst->size  = xmS.size;
                     pInst->loopS = xmS.loopS;
                     pInst->loopE = (xmS.loopS + xmS.loopL);
                 }
@@ -481,7 +495,7 @@
                     if (pInst->loopS >= pInst->size)
                     {
                         JSSWARNING(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
-                        "Samp #%i/%i: loopS >= size (%d >= %d)\n",
+                        "Samp #%d/%d: loopS >= size (%d >= %d)\n",
                         ninst, nsample, pInst->loopS, pInst->size);
                         JSFUNSET(pInst->flags, jsfLooped);
                     }
@@ -489,7 +503,7 @@
                     if (pInst->loopE > pInst->size)
                     {
                         JSSWARNING(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
-                        "Samp #%i/%i: loopE > size (%d > %d)\n",
+                        "Samp #%d/%d: loopE > size (%d > %d)\n",
                         ninst, nsample, pInst->loopE, pInst->size);
                         JSFUNSET(pInst->flags, jsfLooped);
                     }
@@ -504,7 +518,7 @@
 
                 if (pInst->data == NULL)
                     JSSERROR(DMERR_MALLOC, DMERR_MALLOC,
-                    "Could not allocate sample data #%i/%i.\n", ninst, nsample);
+                    "Could not allocate sample data #%d/%d.\n", ninst, nsample);
             }
         }
 
@@ -516,10 +530,10 @@
             if (pInst)
             {
                 JSSDEBUG("desc....: '%s'\n"
-                     "size....: %i\n"
-                     "loopS...: %i\n"
-                     "loopE...: %i\n"
-                     "volume..: %i\n"
+                     "size....: %d\n"
+                     "loopS...: %d\n"
+                     "loopE...: %d\n"
+                     "volume..: %d\n"
                      "flags...: %x\n",
                      pInst->desc,
                      pInst->size, pInst->loopS, pInst->loopE,
@@ -530,7 +544,7 @@
                     // Read sampledata
                     if (dmfread(pInst->data, sizeof(Uint16), pInst->size, inFile) != (size_t) pInst->size)
                         JSSERROR(DMERR_FREAD, DMERR_FREAD,
-                        "Error reading sampledata for instrument #%i/%i, %i words.\n",
+                        "Error reading sampledata for instrument #%d/%d, %d words.\n",
                         ninst, nsample, pInst->size);
 
                     // Convert data
@@ -547,7 +561,7 @@
                     // Read sampledata
                     if (dmfread(pInst->data, sizeof(Uint8), pInst->size, inFile) != (size_t) pInst->size)
                         JSSERROR(DMERR_FREAD, DMERR_FREAD,
-                        "Error reading sampledata for instrument #%i/%i, %i bytes.\n",
+                        "Error reading sampledata for instrument #%d/%d, %d bytes.\n",
                         ninst, nsample, pInst->size);
 
                     // Convert data
@@ -599,23 +613,22 @@
     assert(inFile != NULL);
     *ppModule = NULL;
 
-    /* Get XM-header and check it
-     */
-    dmf_read_str(inFile, &xmH.idMagic, sizeof(xmH.idMagic));
-    dmf_read_str(inFile, &xmH.songName, sizeof(xmH.songName));
-    xmH.unUsed1A = dmfgetc(inFile);
-    dmf_read_str(inFile, &xmH.trackerName, sizeof(xmH.trackerName));
-    dmf_read_le16(inFile, &xmH.version);
-    dmf_read_le32(inFile, &xmH.headSize);
-    dmf_read_le16(inFile, &xmH.norders);
-    dmf_read_le16(inFile, &xmH.defRestartPos);
-    dmf_read_le16(inFile, &xmH.nchannels);
-    dmf_read_le16(inFile, &xmH.npatterns);
-    dmf_read_le16(inFile, &xmH.ninstruments);
-    dmf_read_le16(inFile, &xmH.flags);
-    dmf_read_le16(inFile, &xmH.defSpeed);
-    dmf_read_le16(inFile, &xmH.defTempo);
-    dmf_read_str(inFile, &xmH.orderList, sizeof(xmH.orderList));
+    // Try to get the XM-header
+    if (!dmf_read_str(inFile, &xmH.idMagic, sizeof(xmH.idMagic)) ||
+        !dmf_read_str(inFile, &xmH.songName, sizeof(xmH.songName)) ||
+        !dmf_read_byte(inFile, &xmH.unUsed1A) ||
+        !dmf_read_str(inFile, &xmH.trackerName, sizeof(xmH.trackerName)) ||
+        !dmf_read_le16(inFile, &xmH.version) ||
+        !dmf_read_le32(inFile, &xmH.headSize) ||
+        !dmf_read_le16(inFile, &xmH.norders) ||
+        !dmf_read_le16(inFile, &xmH.defRestartPos) ||
+        !dmf_read_le16(inFile, &xmH.nchannels) ||
+        !dmf_read_le16(inFile, &xmH.npatterns) ||
+        !dmf_read_le16(inFile, &xmH.ninstruments) ||
+        !dmf_read_le16(inFile, &xmH.flags) ||
+        !dmf_read_le16(inFile, &xmH.defSpeed) ||
+        !dmf_read_le16(inFile, &xmH.defTempo))
+        return DMERR_FREAD;
 
 
     // Check the fields, none of these are considered fatal
@@ -641,7 +654,7 @@
     if (xmH.norders > XM_MaxOrders)
     {
         JSSWARNING(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
-        "Number of orders %i > %i, possibly broken module.\n",
+        "Number of orders %d > %d, possibly broken module.\n",
         xmH.norders, XM_MaxOrders);
         xmH.norders = XM_MaxOrders;
     }
@@ -655,7 +668,7 @@
     if (xmH.npatterns > XM_MaxPatterns)
     {
         JSSWARNING(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
-        "Number of patterns %i > %i, possibly broken module.\n",
+        "Number of patterns %d > %d, possibly broken module.\n",
         xmH.npatterns, XM_MaxPatterns);
         xmH.npatterns = XM_MaxPatterns;
     }
@@ -669,17 +682,24 @@
     if (xmH.nchannels <= 0 || xmH.nchannels > XM_MaxChannels)
     {
         JSSWARNING(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
-        "Number of channels was invalid, %i (should be 1 - %i).\n",
+        "Number of channels was invalid, %d (should be 1 - %d).\n",
         xmH.nchannels, XM_MaxChannels);
     }
 
     if (xmH.ninstruments <= 0 || xmH.ninstruments > XM_MaxInstruments)
     {
         JSSWARNING(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
-        "Number of instruments was invalid, %i (should be 1 - %i).\n",
+        "Number of instruments was invalid, %d (should be 1 - %d).\n",
         xmH.ninstruments, XM_MaxInstruments);
     }
 
+    // Read orders list
+    if (!dmf_read_str(inFile, &xmH.orderList, sizeof(xmH.orderList)))
+    {
+        JSSERROR(DMERR_FREAD, DMERR_FREAD,
+        "Error reading pattern order list.\n");
+    }
+
     /* Okay, allocate a module structure
      */
     module = jssAllocateModule();
@@ -724,20 +744,25 @@
 
         // Get the pattern header
         pos = dmftell(inFile);
-        dmf_read_le32(inFile, &xmP.headSize);
-        xmP.packing = dmfgetc(inFile);
-        dmf_read_le16(inFile, &xmP.nrows);
-        dmf_read_le16(inFile, &xmP.size);
+        
+        if (!dmf_read_le32(inFile, &xmP.headSize) ||
+            !dmf_read_byte(inFile, &xmP.packing) ||
+            !dmf_read_le16(inFile, &xmP.nrows) ||
+            !dmf_read_le16(inFile, &xmP.size))
+            JSSERROR(DMERR_FREAD, DMERR_FREAD,
+            "Could not read pattern header data #%d.\n",
+            index);
+        
 
         // Check the header
         if (xmP.packing != 0)
             JSSERROR(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
-            "Pattern #%i packing type unsupported (%i)\n",
+            "Pattern #%d packing type unsupported (%d)\n",
             index, xmP.packing);
 
         if (xmP.nrows == 0)
             JSSERROR(DMERR_INVALID_DATA, DMERR_INVALID_DATA,
-            "Pattern #%i has %i rows, invalid data.\n",
+            "Pattern #%d has %d rows, invalid data.\n",
             index, xmP.nrows);
 
         if (xmP.size > 0)
@@ -746,11 +771,11 @@
             module->patterns[index] = jssAllocatePattern(xmP.nrows, module->nchannels);
             if (module->patterns[index] == NULL)
                 JSSERROR(DMERR_MALLOC, DMERR_MALLOC,
-                "Could not allocate memory for pattern #%i\n", index);
+                "Could not allocate memory for pattern #%d\n", index);
 
             result = jssXMUnpackPattern(inFile, xmP.size, module->patterns[index]);
             if (result != 0)
-                JSSERROR(result, result, "Error in unpacking pattern #%i data\n", index);
+                JSSERROR(result, result, "Error in unpacking pattern #%d data\n", index);
         }
 
         // Skip extra data (if the file is damaged)
@@ -783,7 +808,7 @@
     {
         result = jssXMLoadExtInstrument(inFile, index, module);
         if (result != 0)
-            JSSERROR(result, result, "Errors while reading instrument #%i\n", index);
+            JSSERROR(result, result, "Errors while reading instrument #%d\n", index);
     }
 
     return DMERR_OK;