# HG changeset patch # User Matti Hamalainen # Date 1369348440 -10800 # Node ID a4cf7716ba58fdbfe451e68b837290b19717baa6 # Parent ed60a7ee3ebb7fc8513695cfaf2213a83c858af2 Clean up the XM module loader a bit, add more error checking. diff -r ed60a7ee3ebb -r a4cf7716ba58 minijss/jloadxm.c --- 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;